From 9eee0b6e1a798e373d11ba28813032f86fd048ad Mon Sep 17 00:00:00 2001 From: pshu Date: Sat, 30 May 2026 02:54:13 +0800 Subject: [PATCH 1/8] refactor: use camino Utf8Path for internal path representation Convert the resolver's internal path types to camino's Utf8Path/Utf8PathBuf while keeping every public input and output as std Path/PathBuf. Why: the codebase already assumed UTF-8 paths implicitly (path_to_str's `expect`), so making it explicit at the type level removes scattered OsStr/&str juggling on the hot path and prevents accidental non-UTF-8 handling. Public API is unchanged; conversions happen only at the boundary. --- Cargo.lock | 10 ++ Cargo.toml | 1 + src/cache.rs | 80 +++++++++------- src/lib.rs | 185 ++++++++++++++++++++---------------- src/package_json/simd.rs | 12 ++- src/path.rs | 71 +++++++------- src/resolution.rs | 16 ++-- src/resolver_path.rs | 16 ++++ src/tests/exports_field.rs | 7 +- src/tests/imports_field.rs | 7 +- src/tests/pnp.rs | 18 ++-- src/tests/tsconfig_paths.rs | 23 +++-- src/tests/windows.rs | 8 +- src/tsconfig.rs | 43 ++++----- 14 files changed, 282 insertions(+), 215 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9cb27c5b..eaf6ffca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -103,6 +103,15 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" +[[package]] +name = "camino" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e629a66d692cb9ff1a1c664e41771b3dcaf961985a9774c0eb0bd1b51cf60a48" +dependencies = [ + "serde_core", +] + [[package]] name = "cast" version = "0.3.0" @@ -1021,6 +1030,7 @@ version = "0.9.2" dependencies = [ "arc-swap", "async-trait", + "camino", "cfg-if", "codspeed-criterion-compat", "dashmap", diff --git a/Cargo.toml b/Cargo.toml index b9a87901..a7466831 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ tracing = "0.1.41" simd-json = { version = "0.17.0", features = ["serde_impl", "runtime-detection"], default-features = false } +camino = { version = "1.2.2", features = ["serde1"] } # UTF-8 path types used for all internal path representations cfg-if = "1.0" dunce = "1.0.5" # Normalize Windows paths to the most compatible format, avoiding UNC where possible indexmap = { version = "2.12.0", features = ["serde"] } diff --git a/src/cache.rs b/src/cache.rs index 49d5cd39..21963f93 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -5,10 +5,10 @@ use std::{ hash::{BuildHasherDefault, Hash, Hasher}, io, ops::Deref, - path::{Path, PathBuf}, sync::Arc, }; +use camino::{Utf8Path, Utf8PathBuf}; use dashmap::{DashMap, DashSet}; use futures::future::BoxFuture; use rustc_hash::FxHasher; @@ -26,7 +26,7 @@ use crate::{ pub struct Cache { pub(crate) fs: Fs, paths: DashSet>, - tsconfigs: DashMap, BuildHasherDefault>, + tsconfigs: DashMap, BuildHasherDefault>, } impl Cache { @@ -43,8 +43,8 @@ impl Cache { self.tsconfigs.clear(); } - pub fn value(&self, path: &Path) -> CachedPath { - let hash = hash_path(path); + pub fn value(&self, path: &Utf8Path) -> CachedPath { + let hash = hash_path(path.as_std_path()); if let Some(cache_entry) = self.paths.get((hash, path).borrow() as &dyn CacheKey) { return cache_entry.clone(); } @@ -61,7 +61,7 @@ impl Cache { pub async fn tsconfig( &self, root: bool, - path: &Path, + path: &Utf8Path, callback: F, // callback for modifying tsconfig with `extends` ) -> Result, ResolveError> where @@ -71,25 +71,25 @@ impl Cache { if let Some(tsconfig_ref) = self.tsconfigs.get(path) { return Ok(Arc::clone(tsconfig_ref.value())); } - let meta = self.fs.metadata(path).await.ok(); + let meta = self.fs.metadata(path.as_std_path()).await.ok(); let tsconfig_path = if meta.is_some_and(|m| m.is_file) { Cow::Borrowed(path) } else if meta.is_some_and(|m| m.is_dir) { Cow::Owned(path.join("tsconfig.json")) } else { - let mut os_string = path.to_path_buf().into_os_string(); - os_string.push(".json"); - Cow::Owned(PathBuf::from(os_string)) + let mut string = path.as_str().to_string(); + string.push_str(".json"); + Cow::Owned(Utf8PathBuf::from(string)) }; let mut tsconfig_string = self .fs - .read_to_string(&tsconfig_path) + .read_to_string(tsconfig_path.as_std_path()) .await - .map_err(|_| ResolveError::TsconfigNotFound(path.to_path_buf()))?; + .map_err(|_| ResolveError::TsconfigNotFound(path.as_std_path().to_path_buf()))?; let mut tsconfig = TsConfig::parse(root, &tsconfig_path, &mut tsconfig_string).map_err(|error| { ResolveError::from_serde_json_error( - tsconfig_path.to_path_buf(), + tsconfig_path.as_std_path().to_path_buf(), &error, Some(tsconfig_string), ) @@ -140,17 +140,17 @@ impl AsRef for CachedPath { } impl CacheKey for CachedPath { - fn tuple(&self) -> (u64, &Path) { + fn tuple(&self) -> (u64, &Utf8Path) { (self.hash, &self.path) } } pub struct CachedPathImpl { hash: u64, - path: Box, + path: Box, parent: Option, meta: OnceLock>, - canonicalized: OnceLock>, + canonicalized: OnceLock>, node_modules: OnceLock>, package_json: OnceLock>>, /// Memoized `/package.json` `ResolverPath` for the @@ -164,12 +164,12 @@ impl From<&CachedPathImpl> for ResolverPath { /// only remaining work is one `Arc::from(&Path)` to materialize the shared /// path buffer for the `ResolveContext` sink. fn from(cached: &CachedPathImpl) -> Self { - Self::from_parts(cached.hash, Arc::from(&*cached.path)) + Self::from_parts(cached.hash, Arc::from(cached.path.as_std_path())) } } impl CachedPathImpl { - fn new(hash: u64, path: Box, parent: Option) -> Self { + fn new(hash: u64, path: Box, parent: Option) -> Self { Self { hash, path, @@ -191,11 +191,11 @@ impl CachedPathImpl { .clone() } - pub fn path(&self) -> &Path { + pub fn path(&self) -> &Utf8Path { &self.path } - pub fn to_path_buf(&self) -> PathBuf { + pub fn to_path_buf(&self) -> Utf8PathBuf { self.path.to_path_buf() } @@ -211,7 +211,7 @@ impl CachedPathImpl { } *self .meta - .get_or_init(|| async { fs.metadata(&self.path).await.ok() }) + .get_or_init(|| async { fs.metadata(self.path.as_std_path()).await.ok() }) .await } @@ -235,7 +235,7 @@ impl CachedPathImpl { ) } - pub async fn realpath(&self, fs: &Fs) -> io::Result { + pub async fn realpath(&self, fs: &Fs) -> io::Result { // Cache hit: avoid the heap-allocated `Box::pin` for the cache-miss state machine // by returning before delegating to the boxed recursive helper. if let Some(cached) = self.canonicalized.get() { @@ -247,22 +247,25 @@ impl CachedPathImpl { fn realpath_uncached<'a, Fs: FileSystem + Send + Sync>( &'a self, fs: &'a Fs, - ) -> BoxFuture<'a, io::Result> { + ) -> BoxFuture<'a, io::Result> { Box::pin(async move { self .canonicalized .get_or_try_init(|| async move { if fs - .symlink_metadata(&self.path) + .symlink_metadata(self.path.as_std_path()) .await .is_ok_and(|m| m.is_symlink) { - return fs.canonicalize(&self.path).await.map(Some); + return fs + .canonicalize(self.path.as_std_path()) + .await + .map(|path| Some(Utf8PathBuf::from_path_buf(path).expect("path should be UTF-8"))); } if let Some(parent) = self.parent() { let parent_path = parent.realpath(fs).await?; return Ok(Some( - parent_path.normalize_with(self.path.strip_prefix(&parent.path).unwrap()), + parent_path.normalize_with(self.path.strip_prefix(parent.path()).unwrap()), )); } Ok(None) @@ -311,7 +314,7 @@ impl CachedPathImpl { /// # Errors /// /// * [ResolveError::JSON] - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = %self.path.display())))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = %self.path)))] pub async fn find_package_json( &self, fs: &Fs, @@ -342,7 +345,7 @@ impl CachedPathImpl { /// # Errors /// /// * [ResolveError::JSON] - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = %self.path.display())))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = %self.path)))] pub async fn package_json( &self, fs: &Fs, @@ -366,7 +369,7 @@ impl CachedPathImpl { .package_json .get_or_try_init(|| async { let package_json_path = self.path.join("package.json"); - let Ok(package_json_string) = fs.read(&package_json_path).await else { + let Ok(package_json_string) = fs.read(package_json_path.as_std_path()).await else { return Ok(None); }; let real_path = if options.symlinks { @@ -374,11 +377,16 @@ impl CachedPathImpl { } else { package_json_path.clone() }; - match PackageJson::parse(package_json_path.clone(), real_path, package_json_string) { + match PackageJson::parse( + package_json_path.clone().into_std_path_buf(), + real_path.into_std_path_buf(), + package_json_string, + ) { Ok(v) => Ok(Some(Arc::new(v))), Err(parse_err) => { let package_json_path = self.path.join("package.json"); - let package_json_string = match fs.read_to_string(&package_json_path).await { + let package_json_string = match fs.read_to_string(package_json_path.as_std_path()).await + { Ok(c) => c, Err(io_err) => { return Err(ResolveError::from(io_err)); @@ -388,7 +396,7 @@ impl CachedPathImpl { if let Some(err) = serde_err { Err(ResolveError::from_serde_json_error( - package_json_path, + package_json_path.into_std_path_buf(), &err, Some(package_json_string), )) @@ -396,7 +404,7 @@ impl CachedPathImpl { let (line, column) = off_to_location(&package_json_string, parse_err.index()); Err(ResolveError::JSON(JSONError { - path: package_json_path, + path: package_json_path.into_std_path_buf(), message: parse_err.error().to_string(), line, column, @@ -432,7 +440,7 @@ impl CachedPathImpl { /// Memoized cache key, code adapted from . trait CacheKey { - fn tuple(&self) -> (u64, &Path); + fn tuple(&self) -> (u64, &Utf8Path); } impl Hash for dyn CacheKey + '_ { @@ -449,13 +457,13 @@ impl PartialEq for dyn CacheKey + '_ { impl Eq for dyn CacheKey + '_ {} -impl CacheKey for (u64, &Path) { - fn tuple(&self) -> (u64, &Path) { +impl CacheKey for (u64, &Utf8Path) { + fn tuple(&self) -> (u64, &Utf8Path) { (self.0, self.1) } } -impl<'a> Borrow for (u64, &'a Path) { +impl<'a> Borrow for (u64, &'a Utf8Path) { fn borrow(&self) -> &(dyn CacheKey + 'a) { self } diff --git a/src/lib.rs b/src/lib.rs index 6aca9c1d..0fcfdd7b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,12 +67,12 @@ mod tests; use std::{ borrow::Cow, cmp::Ordering, - ffi::OsStr, fmt, - path::{Component, Path, PathBuf}, + path::{Path, PathBuf}, sync::{Arc, OnceLock}, }; +use camino::{Utf8Component, Utf8Path, Utf8PathBuf}; use dashmap::DashSet; use futures::future::{try_join_all, BoxFuture}; use rustc_hash::FxHashSet; @@ -82,7 +82,7 @@ use crate::{ cache::{Cache, CachedPath}, context::ResolveContext as Ctx, package_json::JSONMap, - path::{path_to_str, PathUtil, SLASH_START}, + path::{PathUtil, SLASH_START}, specifier::Specifier, tsconfig::{ExtendsField, ProjectReference, TsConfig}, }; @@ -262,7 +262,7 @@ impl ResolverGeneric { } /// Wrap `resolve_impl` with `tracing` information - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(directory), specifier = specifier)))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = directory.to_str().expect("path should be UTF-8"), specifier = specifier)))] async fn resolve_tracing( &self, directory: &Path, @@ -290,7 +290,9 @@ impl ResolverGeneric { ctx: &mut Ctx, ) -> Result { ctx.with_fully_specified(self.options.fully_specified); - let cached_path = self.cache.value(path); + let cached_path = self + .cache + .value(Utf8Path::from_path(path).expect("path should be UTF-8")); let cached_path = self.require(&cached_path, specifier, ctx).await?; let path = self.load_realpath(&cached_path, ctx).await?; @@ -299,7 +301,7 @@ impl ResolverGeneric { .await?; if let Some(package_json) = &package_json { // path must be inside the package. - debug_assert!(path.starts_with(package_json.directory())); + debug_assert!(path.as_std_path().starts_with(package_json.directory())); } Ok(Resolution { path, @@ -365,17 +367,17 @@ impl ResolverGeneric { return Ok(path); } - let result = match Path::new(specifier).components().next() { + let result = match Utf8Path::new(specifier).components().next() { // 2. If X begins with '/' - Some(Component::RootDir | Component::Prefix(_)) => { + Some(Utf8Component::RootDir | Utf8Component::Prefix(_)) => { self.require_absolute(cached_path, specifier, ctx).await } // 3. If X begins with './' or '/' or '../' - Some(Component::CurDir | Component::ParentDir) => { + Some(Utf8Component::CurDir | Utf8Component::ParentDir) => { self.require_relative(cached_path, specifier, ctx).await } // 4. If X begins with '#' - Some(Component::Normal(_)) if specifier.as_bytes()[0] == b'#' => { + Some(Utf8Component::Normal(_)) if specifier.as_bytes()[0] == b'#' => { self.require_hash(cached_path, specifier, ctx).await } _ => { @@ -436,10 +438,10 @@ impl ResolverGeneric { ctx: &mut Ctx, ) -> Result { // Make sure only path prefixes gets called - debug_assert!(Path::new(specifier) + debug_assert!(Utf8Path::new(specifier) .components() .next() - .is_some_and(|c| matches!(c, Component::RootDir | Component::Prefix(_)))); + .is_some_and(|c| matches!(c, Utf8Component::RootDir | Utf8Component::Prefix(_)))); if !self.options.prefer_relative && self.options.prefer_absolute { if let Ok(path) = self .load_package_self_or_node_modules(cached_path, specifier, ctx) @@ -456,9 +458,9 @@ impl ResolverGeneric { let path = self.cache.value( // #[cfg(windows)] - &Path::new(specifier).normalize(), + &Utf8Path::new(specifier).normalize(), #[cfg(not(windows))] - Path::new(specifier), + Utf8Path::new(specifier), ); if let Some(path) = self @@ -471,7 +473,7 @@ impl ResolverGeneric { } // 3. If X begins with './' or '/' or '../' - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn require_relative( &self, cached_path: &CachedPath, @@ -479,12 +481,12 @@ impl ResolverGeneric { ctx: &mut Ctx, ) -> Result { // Make sure only relative or normal paths gets called - debug_assert!(Path::new(specifier) + debug_assert!(Utf8Path::new(specifier) .components() .next() .is_some_and(|c| matches!( c, - Component::CurDir | Component::ParentDir | Component::Normal(_) + Utf8Component::CurDir | Utf8Component::ParentDir | Utf8Component::Normal(_) ))); let path = cached_path.path().normalize_with(specifier); let cached_path = self.cache.value(&path); @@ -527,10 +529,10 @@ impl ResolverGeneric { ctx: &mut Ctx, ) -> Result { // Make sure no other path prefixes gets called - debug_assert!(Path::new(specifier) + debug_assert!(Utf8Path::new(specifier) .components() .next() - .is_some_and(|c| matches!(c, Component::Normal(_)))); + .is_some_and(|c| matches!(c, Utf8Component::Normal(_)))); if self.options.prefer_relative { if let Ok(path) = self.require_relative(cached_path, specifier, ctx).await { return Ok(path); @@ -571,7 +573,7 @@ impl ResolverGeneric { Ok((parsed, None)) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn load_package_self_or_node_modules( &self, cached_path: &CachedPath, @@ -625,7 +627,7 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] async fn load_as_file(&self, cached_path: &CachedPath, ctx: &mut Ctx) -> ResolveResult { // enhanced-resolve feature: extension_alias if let Some(path) = self.load_extension_alias(cached_path, ctx).await? { @@ -688,7 +690,7 @@ impl ResolverGeneric { self.load_index(cached_path, ctx).await } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn load_as_file_or_directory( &self, cached_path: &CachedPath, @@ -716,7 +718,7 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path.path().as_str())))] async fn load_extensions( &self, path: &CachedPath, @@ -726,7 +728,7 @@ impl ResolverGeneric { if ctx.fully_specified { return Ok(None); } - let path = path_to_str(path.path()); + let path = path.path().as_str(); // 8 is wild guess for max extension length let mut path_with_extension_buffer = String::with_capacity(path.len() + 8); path_with_extension_buffer.push_str(path); @@ -735,7 +737,7 @@ impl ResolverGeneric { for extension in extensions { path_with_extension_buffer.truncate(base_len); path_with_extension_buffer.push_str(extension); - let cached_path = self.cache.value(Path::new(&path_with_extension_buffer)); + let cached_path = self.cache.value(Utf8Path::new(&path_with_extension_buffer)); if let Some(path) = self.load_alias_or_file(&cached_path, ctx).await? { return Ok(Some(path)); } @@ -743,18 +745,18 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] async fn load_realpath( &self, cached_path: &CachedPath, ctx: &mut Ctx, - ) -> Result { + ) -> Result { if self.options.symlinks { cached_path .realpath(&self.cache.fs) .await .map(|path| { - ctx.add_file_dependency(&path); + ctx.add_file_dependency(path.as_path()); path }) .map_err(ResolveError::from) @@ -763,7 +765,7 @@ impl ResolverGeneric { } } - fn check_restrictions(&self, path: &Path) -> bool { + fn check_restrictions(&self, path: &Utf8Path) -> bool { // https://github.com/webpack/enhanced-resolve/blob/a998c7d218b7a9ec2461fc4fddd1ad5dd7687485/lib/RestrictionsPlugin.js#L19-L24 fn is_inside(path: &Path, parent: &Path) -> bool { if !path.starts_with(parent) { @@ -776,6 +778,7 @@ impl ResolverGeneric { .strip_prefix(parent) .is_ok_and(|p| p == Path::new("./")) } + let path = path.as_std_path(); for restriction in &self.options.restrictions { match restriction { Restriction::Path(restricted_path) => { @@ -793,7 +796,7 @@ impl ResolverGeneric { true } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] async fn load_index(&self, cached_path: &CachedPath, ctx: &mut Ctx) -> ResolveResult { for main_file in &self.options.main_files { let main_path = cached_path.path().normalize_with(main_file); @@ -833,7 +836,7 @@ impl ResolverGeneric { } } // enhanced-resolve: try file as alias - let alias_specifier = path_to_str(cached_path.path()); + let alias_specifier = cached_path.path().as_str(); if let Some(path) = self .load_alias( cached_path, @@ -853,7 +856,7 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn load_node_modules( &self, cached_path: &CachedPath, @@ -982,7 +985,9 @@ impl ResolverGeneric { } let (package_name, subpath) = Self::parse_package_specifier(specifier); for dir in dirs { - let cached_path = self.cache.value(dir); + let cached_path = self + .cache + .value(Utf8Path::from_path(dir).expect("path should be UTF-8")); if !cached_path.is_dir(&self.cache.fs, ctx).await { continue; } @@ -997,7 +1002,7 @@ impl ResolverGeneric { } #[cfg(feature = "yarn_pnp")] - #[cfg_attr(feature = "enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature = "enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] fn find_pnp_manifest(&self, cached_path: &CachedPath) -> Option> { // 1. Already have a manifest → return it (covers global cache paths too) if let Some(manifest) = self.pnp_manifest.load_full() { @@ -1011,7 +1016,7 @@ impl ResolverGeneric { // 3. Search for `.pnp.cjs` by walking up from this path let base_path = cached_path.to_path_buf(); - let Some(manifest_path) = pnp::find_closest_pnp_manifest_path(&base_path) else { + let Some(manifest_path) = pnp::find_closest_pnp_manifest_path(base_path.as_std_path()) else { self.pnp_no_manifest_cache.insert(cached_path.clone()); for p in base_path.ancestors() { @@ -1041,7 +1046,7 @@ impl ResolverGeneric { } #[cfg(feature = "yarn_pnp")] - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn load_pnp( &self, cached_path: &CachedPath, @@ -1056,13 +1061,16 @@ impl ResolverGeneric { let mut path = cached_path.to_path_buf(); path.push(""); - let resolution = pnp::resolve_to_unqualified_via_manifest(manifest, specifier, &path); + let resolution = + pnp::resolve_to_unqualified_via_manifest(manifest, specifier, path.as_std_path()); tracing::debug!("pnp resolve unqualified as: {:?}", resolution); match resolution { Ok(pnp::Resolution::Resolved(path, subpath)) => { - let cached_path = self.cache.value(&path); + let cached_path = self + .cache + .value(Utf8Path::from_path(&path).expect("path should be UTF-8")); let export_resolution = self.load_package_self(&cached_path, specifier, ctx).await?; // can be found in pnp cached folder @@ -1090,7 +1098,9 @@ impl ResolverGeneric { return Err(ResolveError::NotFound(specifier.to_string())); }; - Ok(Some(self.cache.value(inner_resolution.path()))) + Ok(Some(self.cache.value( + Utf8Path::from_path(inner_resolution.path()).expect("path should be UTF-8"), + ))) } Ok(pnp::Resolution::Skipped) => Ok(None), @@ -1107,7 +1117,7 @@ impl ResolverGeneric { if module_name == "node_modules" { cached_path.cached_node_modules(&self.cache, ctx).await } else if cached_path.path().components().next_back() - == Some(Component::Normal(OsStr::new(module_name))) + == Some(Utf8Component::Normal(module_name)) { Some(cached_path.clone()) } else { @@ -1149,7 +1159,7 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn load_package_self( &self, cached_path: &CachedPath, @@ -1174,7 +1184,8 @@ impl ResolverGeneric { // 5. let MATCH = PACKAGE_EXPORTS_RESOLVE(pathToFileURL(SCOPE), // "." + X.slice("name".length), `package.json` "exports", ["node", "require"]) // defined in the ESM resolver. - let package_url = package_json.directory(); + let package_url = + Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8"); // Note: The subpath is not prepended with a dot on purpose // because `package_exports_resolve` matches subpath without the leading dot. for exports in package_json.exports_fields(&self.options.exports_fields) { @@ -1193,7 +1204,7 @@ impl ResolverGeneric { } /// RESOLVE_ESM_MATCH(MATCH) - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = specifier, path = cached_path.path().as_str())))] async fn resolve_esm_match( &self, specifier: &str, @@ -1208,7 +1219,7 @@ impl ResolverGeneric { return Ok(Some(path)); } - let mut path_str = cached_path.path().to_str(); + let mut path_str = Some(cached_path.path().as_str()); // 3. If the RESOLVED_PATH contains `?``, it could be a path with query // so try to resolve it as a file or directory without the query, @@ -1216,7 +1227,7 @@ impl ResolverGeneric { while let Some(s) = path_str { if let Some((before, _)) = s.rsplit_once('?') { if (self - .load_as_file_or_directory(&self.cache.value(Path::new(before)), "", ctx) + .load_as_file_or_directory(&self.cache.value(Utf8Path::new(before)), "", ctx) .await?) .is_some() { @@ -1233,7 +1244,7 @@ impl ResolverGeneric { } /// enhanced-resolve: AliasFieldPlugin for [ResolveOptions::alias_fields] - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = module_specifier, path = path_to_str(cached_path.path()))))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(specifier = module_specifier, path = cached_path.path().as_str())))] async fn load_browser_field( &self, cached_path: &CachedPath, @@ -1259,7 +1270,7 @@ impl ResolverGeneric { // Complete when resolving to self `{"./a.js": "./a.js"}` if new_specifier .strip_prefix("./") - .filter(|s| path.ends_with(Path::new(s))) + .filter(|s| path.ends_with(s)) .is_some() { return if cached_path.is_file(&self.cache.fs, ctx).await { @@ -1276,7 +1287,9 @@ impl ResolverGeneric { } ctx.with_resolving_alias(new_specifier.to_string()); ctx.with_fully_specified(false); - let cached_path = self.cache.value(package_json.directory()); + let cached_path = self + .cache + .value(Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8")); self .require(&cached_path, new_specifier, ctx) .await @@ -1324,7 +1337,7 @@ impl ResolverGeneric { } AliasValue::Ignore => { let path = cached_path.path().normalize_with(alias_key); - return Err(ResolveError::Ignored(path)); + return Err(ResolveError::Ignored(path.into_std_path_buf())); } } } @@ -1357,7 +1370,7 @@ impl ResolverGeneric { let new_specifier = if tail.is_empty() { Cow::Borrowed(alias_value) } else { - let alias_path = Path::new(alias_value).normalize(); + let alias_path = Utf8Path::new(alias_value).normalize(); // Must not append anything to alias_value if it is a file. let alias_value_cached_path = self.cache.value(&alias_path); if alias_value_cached_path.is_file(&self.cache.fs, ctx).await { @@ -1370,7 +1383,7 @@ impl ResolverGeneric { Cow::Borrowed(alias_value) } else { let normalized = alias_path.normalize_with(tail); - Cow::Owned(path_to_str(&normalized).to_string()) + Cow::Owned(normalized.into_string()) } }; @@ -1404,7 +1417,7 @@ impl ResolverGeneric { .options .extension_alias .iter() - .find(|(ext, _)| OsStr::new(ext.trim_start_matches('.')) == path_extension) + .find(|(ext, _)| ext.trim_start_matches('.') == path_extension) else { return Ok(None); }; @@ -1416,10 +1429,10 @@ impl ResolverGeneric { ctx.with_fully_specified(true); for extension in extensions { - let mut path_with_extension = path_without_extension.clone().into_os_string(); + let mut path_with_extension = path_without_extension.clone().into_string(); path_with_extension.reserve_exact(extension.len()); - path_with_extension.push(extension); - let cached_path = self.cache.value(Path::new(&path_with_extension)); + path_with_extension.push_str(extension); + let cached_path = self.cache.value(Utf8Path::new(&path_with_extension)); if let Some(path) = self.load_alias_or_file(&cached_path, ctx).await? { ctx.with_fully_specified(false); return Ok(Some(path)); @@ -1433,16 +1446,16 @@ impl ResolverGeneric { return Ok(None); } // Create a meaningful error message. - let dir = path.parent().unwrap().to_path_buf(); - let filename_without_extension = Path::new(filename).with_extension(""); - let filename_without_extension = path_to_str(&filename_without_extension); + let dir = path.parent().unwrap().as_std_path().to_path_buf(); + let filename_without_extension = Utf8Path::new(filename).with_extension(""); + let filename_without_extension = filename_without_extension.as_str(); let files = extensions .iter() .map(|ext| format!("{filename_without_extension}{ext}")) .collect::>() .join(","); Err(ResolveError::ExtensionAlias( - filename.to_str().expect("path should be UTF-8").to_string(), + filename.to_string(), files, dir, )) @@ -1460,7 +1473,9 @@ impl ResolverGeneric { } if let Some(specifier) = specifier.strip_prefix(SLASH_START) { for root in &self.options.roots { - let cached_path = self.cache.value(root); + let cached_path = self + .cache + .value(Utf8Path::from_path(root).expect("path should be UTF-8")); if let Ok(path) = self.require_relative(&cached_path, specifier, ctx).await { return Some(path); } @@ -1481,12 +1496,12 @@ impl ResolverGeneric { let tsconfig = self .load_tsconfig( /* root */ true, - &tsconfig_options.config_file, + Utf8Path::from_path(&tsconfig_options.config_file).expect("path should be UTF-8"), &tsconfig_options.references, ) .await?; for dependency in &tsconfig.file_dependencies { - ctx.add_file_dependency(dependency); + ctx.add_file_dependency(dependency.as_path()); } let paths = tsconfig.resolve(cached_path.path(), specifier); for path in paths { @@ -1498,11 +1513,11 @@ impl ResolverGeneric { Ok(None) } - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip(self), fields(path = path.display().to_string())))] + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip(self), fields(path = path.as_str())))] fn load_tsconfig<'a>( &'a self, root: bool, - path: &'a Path, + path: &'a Utf8Path, references: &'a TsconfigReferences, ) -> BoxFuture<'a, Result, ResolveError>> { let fut = async move { @@ -1527,7 +1542,7 @@ impl ResolverGeneric { tsconfig.references = paths .iter() .map(|path| ProjectReference { - path: path.clone(), + path: Utf8PathBuf::from_path_buf(path.clone()).expect("path should be UTF-8"), tsconfig: None, }) .collect(); @@ -1555,7 +1570,7 @@ impl ResolverGeneric { fn load_references<'a>( &'a self, tsconfig: &'a mut TsConfig, - visited: &'a mut FxHashSet, + visited: &'a mut FxHashSet, ) -> BoxFuture<'a, Result<(), ResolveError>> { Box::pin(async move { if tsconfig.references.is_empty() { @@ -1569,7 +1584,7 @@ impl ResolverGeneric { // Reborrow so the closure below can capture `&mut FxHashSet` across // multiple iterations of this loop. Without the reborrow, the first // iteration would consume the original `&mut visited` binding. - let visited: &mut FxHashSet = &mut *visited; + let visited: &mut FxHashSet = &mut *visited; let reference_tsconfig = self .cache .tsconfig( @@ -1580,7 +1595,7 @@ impl ResolverGeneric { async move { if reference_tsconfig.path == current_path { return Err(ResolveError::TsconfigSelfReference( - reference_tsconfig.path.clone(), + reference_tsconfig.path.clone().into_std_path_buf(), )); } // Cut the cycle: if this reference is already part of the @@ -1655,12 +1670,12 @@ impl ResolverGeneric { directory: &CachedPath, tsconfig: &TsConfig, specifier: &str, - ) -> Result { + ) -> Result { match specifier.as_bytes().first() { None => Err(ResolveError::Specifier(SpecifierError::Empty( specifier.to_string(), ))), - Some(b'/') => Ok(PathBuf::from(specifier)), + Some(b'/') => Ok(Utf8PathBuf::from(specifier)), Some(b'.') => Ok(tsconfig.directory().normalize_with(specifier)), _ => self .clone_with_options(ResolveOptions { @@ -1713,7 +1728,9 @@ impl ResolverGeneric { // Fallback: search NODE_PATH directories for dir in self.node_path_dirs() { - let cached_path = self.cache.value(dir); + let cached_path = self + .cache + .value(Utf8Path::from_path(dir).expect("path should be UTF-8")); if !cached_path.is_dir(&self.cache.fs, ctx).await { continue; } @@ -1773,7 +1790,7 @@ impl ResolverGeneric { /// PACKAGE_EXPORTS_RESOLVE(packageURL, subpath, exports, conditions) fn package_exports_resolve<'a>( &'a self, - package_url: &'a Path, + package_url: &'a Utf8Path, subpath: &'a str, exports: &'a JSONValue, ctx: &'a mut Ctx, @@ -1790,7 +1807,7 @@ impl ResolverGeneric { without_dot = without_dot || !starts_with_dot_or_hash; if has_dot && without_dot { return Err(ResolveError::InvalidPackageConfig( - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } } @@ -1806,7 +1823,7 @@ impl ResolverGeneric { let fragment = ctx.fragment.clone().unwrap_or_default(); return Err(ResolveError::PackagePathNotExported( format!("./{}{query}{fragment}", subpath.trim_start_matches('.')), - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } // 1. Let mainExport be undefined. @@ -1879,7 +1896,7 @@ impl ResolverGeneric { // 4. Throw a Package Path Not Exported error. Err(ResolveError::PackagePathNotExported( subpath.to_string(), - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )) }; Box::pin(fut) @@ -1919,7 +1936,7 @@ impl ResolverGeneric { .package_imports_exports_resolve( specifier, imports, - package_json.directory(), + Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8"), /* is_imports */ true, &self.options.condition_names, ctx, @@ -1947,7 +1964,7 @@ impl ResolverGeneric { &self, match_key: &str, match_obj: &JSONMap<'_>, - package_url: &Path, + package_url: &Utf8Path, is_imports: bool, conditions: &[String], ctx: &mut Ctx, @@ -2034,7 +2051,7 @@ impl ResolverGeneric { #[allow(clippy::too_many_arguments)] fn package_target_resolve<'a>( &'a self, - package_url: &'a Path, + package_url: &'a Utf8Path, target_key: &'a str, target: &'a JSONValue, pattern_match: Option<&'a str>, @@ -2047,7 +2064,7 @@ impl ResolverGeneric { target_key: &'a str, target: &'a str, pattern_match: Option<&'a str>, - package_url: &Path, + package_url: &Utf8Path, ) -> Result, ResolveError> { let target = if let Some(pattern_match) = pattern_match { if !target_key.contains('*') && !target.contains('*') { @@ -2057,7 +2074,7 @@ impl ResolverGeneric { Cow::Owned(format!("{target}{pattern_match}")) } else { return Err(ResolveError::InvalidPackageConfigDirectory( - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } } else { @@ -2080,7 +2097,7 @@ impl ResolverGeneric { return Err(ResolveError::InvalidPackageTarget( target.to_string(), target_key.to_string(), - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } // 2. If patternMatch is a String, then @@ -2096,11 +2113,11 @@ impl ResolverGeneric { // 4. Assert: resolvedTarget is contained in packageURL. // 5. If patternMatch is null, then let target = normalize_string_target(target_key, target, pattern_match, package_url)?; - if Path::new(target.as_ref()).is_invalid_exports_target() { + if Utf8Path::new(target.as_ref()).is_invalid_exports_target() { return Err(ResolveError::InvalidPackageTarget( target.to_string(), target_key.to_string(), - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } let resolved_target = package_url.normalize_with(target.as_ref()); @@ -2147,7 +2164,7 @@ impl ResolverGeneric { // Note: return PackagePathNotExported has the same effect as return because there are no matches. return Err(ResolveError::PackagePathNotExported( pattern_match.unwrap_or(".").to_string(), - package_url.join("package.json"), + package_url.join("package.json").into_std_path_buf(), )); } // 2. For each item targetValue in target, do diff --git a/src/package_json/simd.rs b/src/package_json/simd.rs index e779b571..e2c1c3d3 100644 --- a/src/package_json/simd.rs +++ b/src/package_json/simd.rs @@ -8,6 +8,7 @@ use std::{ path::{Path, PathBuf}, }; +use camino::Utf8Path; use simd_json::{ borrowed::Value, prelude::*, to_borrowed_value, BorrowedValue, Error as SimdParseError, }; @@ -308,7 +309,7 @@ impl PackageJson { /// * Returns [ResolveError::Ignored] for `"path": false` in `browser` field. pub(crate) fn resolve_browser_field<'a>( &'a self, - path: &Path, + path: &Utf8Path, request: Option<&str>, alias_fields: &'a [Vec], ) -> Result, ResolveError> { @@ -318,7 +319,7 @@ impl PackageJson { return Self::alias_value(path, value); } } else { - let dir = self.path.parent().unwrap(); + let dir = Utf8Path::from_path(self.path.parent().unwrap()).expect("path should be UTF-8"); for (key, value) in object { let joined = dir.normalize_with(key.to_string()); if joined == path { @@ -330,12 +331,15 @@ impl PackageJson { Ok(None) } - fn alias_value<'a>(key: &Path, value: &'a JSONValue) -> Result, ResolveError> { + fn alias_value<'a>( + key: &Utf8Path, + value: &'a JSONValue, + ) -> Result, ResolveError> { match value { JSONValue::String(value) => Ok(Some(value)), JSONValue::Static(sn) => { if matches!(sn.as_bool(), Some(false)) { - Err(ResolveError::Ignored(key.to_path_buf())) + Err(ResolveError::Ignored(key.as_std_path().to_path_buf())) } else { Ok(None) } diff --git a/src/path.rs b/src/path.rs index 5034533d..ad9b7d77 100644 --- a/src/path.rs +++ b/src/path.rs @@ -3,58 +3,54 @@ //! Code adapted from the following libraries //! * [path-absolutize](https://docs.rs/path-absolutize) //! * [normalize_path](https://docs.rs/normalize-path) -use std::path::{Component, Path, PathBuf}; +use camino::{Utf8Component, Utf8Path, Utf8PathBuf}; pub const SLASH_START: &[char; 2] = &['/', '\\']; -pub fn path_to_str(path: &Path) -> &str { - path.to_str().expect("path should be UTF-8") -} - -/// Extension trait to add path normalization to std's [`Path`]. +/// Extension trait to add path normalization to camino's [`Utf8Path`]. pub trait PathUtil { /// Normalize this path without performing I/O. /// /// All redundant separator and up-level references are collapsed. /// /// However, this does not resolve links. - fn normalize(&self) -> PathBuf; + fn normalize(&self) -> Utf8PathBuf; /// Normalize with subpath assuming this path is normalized without performing I/O. /// /// All redundant separator and up-level references are collapsed. /// /// However, this does not resolve links. - fn normalize_with>(&self, subpath: P) -> PathBuf; + fn normalize_with>(&self, subpath: P) -> Utf8PathBuf; /// Defined in ESM PACKAGE_TARGET_RESOLVE /// If target split on "/" or "\" contains any "", ".", "..", or "node_modules" segments after the first "." segment, case insensitive and including percent encoded variants fn is_invalid_exports_target(&self) -> bool; } -impl PathUtil for Path { +impl PathUtil for Utf8Path { // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L7 - fn normalize(&self) -> PathBuf { + fn normalize(&self) -> Utf8PathBuf { let mut components = self.components().peekable(); - let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek() { - let buf = PathBuf::from(c.as_os_str()); + let mut ret = if let Some(c @ Utf8Component::Prefix(..)) = components.peek() { + let buf = Utf8PathBuf::from(c.as_str()); components.next(); buf } else { - PathBuf::new() + Utf8PathBuf::new() }; for component in components { match component { - Component::Prefix(..) => unreachable!("Path {:?}", self), - Component::RootDir => { - ret.push(component.as_os_str()); + Utf8Component::Prefix(..) => unreachable!("Path {:?}", self), + Utf8Component::RootDir => { + ret.push(component.as_str()); } - Component::CurDir => {} - Component::ParentDir => { + Utf8Component::CurDir => {} + Utf8Component::ParentDir => { ret.pop(); } - Component::Normal(c) => { + Utf8Component::Normal(c) => { ret.push(c); } } @@ -64,7 +60,7 @@ impl PathUtil for Path { } // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L37 - fn normalize_with>(&self, subpath: B) -> PathBuf { + fn normalize_with>(&self, subpath: B) -> Utf8PathBuf { let subpath = subpath.as_ref(); let mut components = subpath.components(); @@ -73,21 +69,21 @@ impl PathUtil for Path { return subpath.to_path_buf(); }; - if matches!(head, Component::Prefix(..) | Component::RootDir) { + if matches!(head, Utf8Component::Prefix(..) | Utf8Component::RootDir) { return subpath.to_path_buf(); } let mut ret = self.to_path_buf(); for component in std::iter::once(head).chain(components) { match component { - Component::CurDir => {} - Component::ParentDir => { + Utf8Component::CurDir => {} + Utf8Component::ParentDir => { ret.pop(); } - Component::Normal(c) => { + Utf8Component::Normal(c) => { ret.push(c); } - Component::Prefix(..) | Component::RootDir => { + Utf8Component::Prefix(..) | Utf8Component::RootDir => { unreachable!("Path {:?} Subpath {:?}", self, subpath) } } @@ -98,9 +94,9 @@ impl PathUtil for Path { fn is_invalid_exports_target(&self) -> bool { self.components().enumerate().any(|(index, c)| match c { - Component::ParentDir => true, - Component::CurDir => index > 0, - Component::Normal(c) => c.eq_ignore_ascii_case("node_modules"), + Utf8Component::ParentDir => true, + Utf8Component::CurDir => index > 0, + Utf8Component::Normal(c) => c.eq_ignore_ascii_case("node_modules"), _ => false, }) } @@ -122,20 +118,23 @@ async fn is_invalid_exports_target() { ]; for case in test_cases { - assert!(Path::new(case).is_invalid_exports_target(), "{case}"); + assert!(Utf8Path::new(case).is_invalid_exports_target(), "{case}"); } - assert!(!Path::new("C:").is_invalid_exports_target()); - assert!(!Path::new("/").is_invalid_exports_target()); + assert!(!Utf8Path::new("C:").is_invalid_exports_target()); + assert!(!Utf8Path::new("/").is_invalid_exports_target()); } #[tokio::test] async fn normalize() { - assert_eq!(Path::new("/foo/.././foo/").normalize(), Path::new("/foo")); - assert_eq!(Path::new("C://").normalize(), Path::new("C://")); - assert_eq!(Path::new("C:").normalize(), Path::new("C:")); assert_eq!( - Path::new(r"\\server\share").normalize(), - Path::new(r"\\server\share") + Utf8Path::new("/foo/.././foo/").normalize(), + Utf8Path::new("/foo") + ); + assert_eq!(Utf8Path::new("C://").normalize(), Utf8Path::new("C://")); + assert_eq!(Utf8Path::new("C:").normalize(), Utf8Path::new("C:")); + assert_eq!( + Utf8Path::new(r"\\server\share").normalize(), + Utf8Path::new(r"\\server\share") ); } diff --git a/src/resolution.rs b/src/resolution.rs index 32deb8b3..a6e949d7 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -4,12 +4,14 @@ use std::{ sync::Arc, }; +use camino::Utf8PathBuf; + use crate::package_json::PackageJson; /// The final path resolution with optional `?query` and `#fragment` #[derive(Clone)] pub struct Resolution { - pub(crate) path: PathBuf, + pub(crate) path: Utf8PathBuf, /// path query `?query`, contains `?`. pub(crate) query: Option, @@ -41,12 +43,12 @@ impl Eq for Resolution {} impl Resolution { /// Returns the path without query and fragment pub fn path(&self) -> &Path { - &self.path + self.path.as_std_path() } /// Returns the path without query and fragment pub fn into_path_buf(self) -> PathBuf { - self.path + self.path.into_std_path_buf() } /// Returns the path query `?query`, contains the leading `?` @@ -66,12 +68,12 @@ impl Resolution { /// Returns the full path with query and fragment pub fn full_path(&self) -> PathBuf { - let mut path = self.path.clone().into_os_string(); + let mut path = self.path.clone().into_string(); if let Some(query) = &self.query { - path.push(query); + path.push_str(query); } if let Some(fragment) = &self.fragment { - path.push(fragment); + path.push_str(fragment); } PathBuf::from(path) } @@ -80,7 +82,7 @@ impl Resolution { #[tokio::test] async fn test() { let resolution = Resolution { - path: PathBuf::from("foo"), + path: Utf8PathBuf::from("foo"), query: Some("?query".to_string()), fragment: Some("#fragment".to_string()), package_json: None, diff --git a/src/resolver_path.rs b/src/resolver_path.rs index 68c13ac1..45e3883f 100644 --- a/src/resolver_path.rs +++ b/src/resolver_path.rs @@ -8,6 +8,7 @@ use std::{ sync::Arc, }; +use camino::{Utf8Path, Utf8PathBuf}; use rustc_hash::FxHasher; /// A path returned in [`crate::ResolveContext`] dependencies, paired with a @@ -150,6 +151,21 @@ impl From> for ResolverPath { } } +// The resolver stores paths internally as UTF-8 (`camino`); these accept the +// internal representation directly while keeping the stored buffer an +// `Arc` so the `as_arc`/`into_arc` output contract stays unchanged. +impl From for ResolverPath { + fn from(path: Utf8PathBuf) -> Self { + Self::new(Arc::from(path.into_std_path_buf())) + } +} + +impl From<&Utf8Path> for ResolverPath { + fn from(path: &Utf8Path) -> Self { + Self::new(Arc::from(path.as_std_path())) + } +} + impl fmt::Debug for ResolverPath { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.path.fmt(f) diff --git a/src/tests/exports_field.rs b/src/tests/exports_field.rs index cfbbc8b1..e63f4d64 100644 --- a/src/tests/exports_field.rs +++ b/src/tests/exports_field.rs @@ -2,8 +2,7 @@ //! //! The huge exports field test cases are at the bottom of this file. -use std::path::Path; - +use camino::Utf8Path; use simd_json::json; use crate::{package_json, Ctx, PathUtil, ResolveError, ResolveOptions, Resolver}; @@ -2604,7 +2603,7 @@ async fn test_cases() { ..ResolveOptions::default() }) .package_exports_resolve( - Path::new(""), + Utf8Path::new(""), case.request, &case.exports_field, &mut Ctx::default(), @@ -2623,7 +2622,7 @@ async fn test_cases() { for expect in expect { assert_eq!( resolved, - Ok(Some(Path::new(expect).normalize())), + Ok(Some(Utf8Path::new(expect).normalize())), "{}", &case.name ); diff --git a/src/tests/imports_field.rs b/src/tests/imports_field.rs index 6c9727a1..135c5294 100644 --- a/src/tests/imports_field.rs +++ b/src/tests/imports_field.rs @@ -2,8 +2,7 @@ //! //! The huge imports field test cases are at the bottom of this file. -use std::path::Path; - +use camino::Utf8Path; use simd_json::{json, prelude::*}; use crate::{package_json::JSONValue, Ctx, PathUtil, ResolveError, ResolveOptions, Resolver}; @@ -1308,7 +1307,7 @@ async fn test_cases() { .package_imports_exports_resolve( case.request, case.imports_field.as_object().unwrap(), - Path::new(""), + Utf8Path::new(""), true, &case .condition_names @@ -1331,7 +1330,7 @@ async fn test_cases() { for expect in expect { assert_eq!( resolved, - Ok(Some(Path::new(expect).normalize())), + Ok(Some(Utf8Path::new(expect).normalize())), "{}", &case.name ); diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index b1320970..0e1afcc6 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -3,6 +3,8 @@ //! enhanced_resolve's test //! cannot be ported over because it uses mocks on `pnpApi` provided by the runtime. +use camino::Utf8Path; + use crate::{ path::PathUtil, ResolveContext, ResolveError::NotFound, ResolveOptions, Resolver, ResolverPath, }; @@ -154,13 +156,15 @@ async fn pnp_resolve_description_file() { .to_str() .expect("path should be UTF-8") .to_string(), - fixture - .join(".yarn/cache/preact-npm-10.25.4-2dd2c0aa44-33a009d614.zip/node_modules/preact") - .join("package.json") - .normalize() - .to_str() - .expect("path should be UTF-8") - .to_string() + Utf8Path::from_path( + &fixture + .join(".yarn/cache/preact-npm-10.25.4-2dd2c0aa44-33a009d614.zip/node_modules/preact") + .join("package.json"), + ) + .expect("path should be UTF-8") + .normalize() + .as_str() + .to_string() ); } diff --git a/src/tests/tsconfig_paths.rs b/src/tests/tsconfig_paths.rs index 10d51fe4..8237b5a5 100644 --- a/src/tests/tsconfig_paths.rs +++ b/src/tests/tsconfig_paths.rs @@ -2,7 +2,7 @@ //! //! Fixtures copied from . -use std::path::{Path, PathBuf}; +use camino::{Utf8Path, Utf8PathBuf}; use crate::{ JSONError, ResolveError, ResolveOptions, Resolver, TsConfig, TsconfigOptions, TsconfigReferences, @@ -138,7 +138,7 @@ async fn empty() { // #[tokio::test] async fn test_paths() { - let path = Path::new("/foo/tsconfig.json"); + let path = Utf8Path::new("/foo/tsconfig.json"); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "paths": { @@ -167,7 +167,10 @@ async fn test_paths() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected.into_iter().map(PathBuf::from).collect::>(); + let expected = expected + .into_iter() + .map(Utf8PathBuf::from) + .collect::>(); assert_eq!(paths, expected, "{specifier}"); } } @@ -175,7 +178,7 @@ async fn test_paths() { // #[tokio::test] async fn test_base_url() { - let path = Path::new("/foo/tsconfig.json"); + let path = Utf8Path::new("/foo/tsconfig.json"); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "baseUrl": "./src" @@ -192,7 +195,10 @@ async fn test_base_url() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected.into_iter().map(PathBuf::from).collect::>(); + let expected = expected + .into_iter() + .map(Utf8PathBuf::from) + .collect::>(); assert_eq!(paths, expected, "{specifier}"); } } @@ -200,7 +206,7 @@ async fn test_base_url() { // #[tokio::test] async fn test_paths_and_base_url() { - let path = Path::new("/foo/tsconfig.json"); + let path = Utf8Path::new("/foo/tsconfig.json"); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "baseUrl": "./src", @@ -235,7 +241,10 @@ async fn test_paths_and_base_url() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected.into_iter().map(PathBuf::from).collect::>(); + let expected = expected + .into_iter() + .map(Utf8PathBuf::from) + .collect::>(); assert_eq!(paths, expected, "{specifier}"); } } diff --git a/src/tests/windows.rs b/src/tests/windows.rs index cd09bc98..8a7989e7 100644 --- a/src/tests/windows.rs +++ b/src/tests/windows.rs @@ -2,6 +2,8 @@ mod tests { use std::{collections::HashSet, path::Path}; + use camino::Utf8Path; + use crate::{path::PathUtil, tests::fixture, FileSystemOs, ResolverGeneric}; #[tokio::test] @@ -52,10 +54,10 @@ mod tests { } fn to_string>(p: P) -> String { - p.as_ref() - .normalize() - .to_str() + Utf8Path::from_path(p.as_ref()) .expect("path should be UTF-8") + .normalize() + .as_str() .to_string() } } diff --git a/src/tsconfig.rs b/src/tsconfig.rs index e1545992..b432349a 100644 --- a/src/tsconfig.rs +++ b/src/tsconfig.rs @@ -1,14 +1,11 @@ -use std::{ - hash::BuildHasherDefault, - path::{Path, PathBuf}, - sync::Arc, -}; +use std::{hash::BuildHasherDefault, sync::Arc}; +use camino::{Utf8Path, Utf8PathBuf}; use indexmap::IndexMap; use rustc_hash::FxHasher; use serde::Deserialize; -use crate::path::{path_to_str, PathUtil}; +use crate::path::PathUtil; pub type CompilerOptionsPathsMap = IndexMap, BuildHasherDefault>; @@ -31,10 +28,10 @@ pub struct TsConfig { /// Path to `tsconfig.json`. Contains the `tsconfig.json` filename. #[serde(skip)] - pub(crate) path: PathBuf, + pub(crate) path: Utf8PathBuf, #[serde(skip)] - pub(crate) file_dependencies: Vec, + pub(crate) file_dependencies: Vec, #[serde(default)] pub extends: Option, @@ -53,14 +50,14 @@ pub struct TsConfig { #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CompilerOptions { - base_url: Option, + base_url: Option, /// Path aliases paths: Option, /// The actual base for where path aliases are resolved from. #[serde(skip)] - paths_base: PathBuf, + paths_base: Utf8PathBuf, } /// Project Reference @@ -70,7 +67,7 @@ pub struct CompilerOptions { pub struct ProjectReference { /// The path property of each reference can point to a directory containing a tsconfig.json file, /// or to the config file itself (which may have any name). - pub path: PathBuf, + pub path: Utf8PathBuf, /// Reference to the resolved tsconfig #[serde(skip)] @@ -78,8 +75,8 @@ pub struct ProjectReference { } impl TsConfig { - #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path_to_str(path))))] - pub fn parse(root: bool, path: &Path, json: &mut str) -> Result { + #[cfg_attr(feature="enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = path.as_str())))] + pub fn parse(root: bool, path: &Utf8Path, json: &mut str) -> Result { _ = json_strip_comments::strip(json); if json.trim().is_empty() { let mut tsconfig: Self = serde_json::from_str("{}")?; @@ -121,12 +118,12 @@ impl TsConfig { } } - let mut p = path_to_str(&self.compiler_options.paths_base).to_string(); + let mut p = self.compiler_options.paths_base.as_str().to_string(); Self::substitute_template_variable(&dir, &mut p); self.compiler_options.paths_base = p.into(); if let Some(base_url) = self.compiler_options.base_url.as_mut() { - let mut p = path_to_str(base_url).to_string(); + let mut p = base_url.as_str().to_string(); Self::substitute_template_variable(&dir, &mut p); *base_url = p.into(); } @@ -139,7 +136,7 @@ impl TsConfig { /// # Panics /// /// * When the `tsconfig.json` path is misconfigured. - pub fn directory(&self) -> &Path { + pub fn directory(&self) -> &Utf8Path { debug_assert!(self.path.file_name().is_some()); self.path.parent().unwrap() } @@ -165,7 +162,7 @@ impl TsConfig { .extend(other_config.file_dependencies.iter().cloned()); } - pub fn resolve(&self, path: &Path, specifier: &str) -> Vec { + pub fn resolve(&self, path: &Utf8Path, specifier: &str) -> Vec { if let Some(matched) = self.find_reference_paths(path, specifier) { return matched; } @@ -177,7 +174,7 @@ impl TsConfig { // (A → B → C): a file inside C should resolve via C's own `paths` even // when the entry tsconfig is A and only B is listed directly in A's // references. Matches `tsc`'s "nearest tsconfig wins" semantics. - fn find_reference_paths(&self, path: &Path, specifier: &str) -> Option> { + fn find_reference_paths(&self, path: &Utf8Path, specifier: &str) -> Option> { for tsconfig in self .references .iter() @@ -195,7 +192,7 @@ impl TsConfig { // Copied from parcel // - pub fn resolve_path_alias(&self, specifier: &str) -> Vec { + pub fn resolve_path_alias(&self, specifier: &str) -> Vec { if specifier.starts_with(['/', '.']) { return vec![]; } @@ -256,7 +253,7 @@ impl TsConfig { .collect() } - fn base_path(&self) -> &Path { + fn base_path(&self) -> &Utf8Path { self .compiler_options .base_url @@ -269,12 +266,12 @@ impl TsConfig { /// NOTE: All tests cases are just a head replacement of `${configDir}`, so we are constrained as such. /// /// See - fn substitute_template_variable(directory: &Path, path: &mut String) { + fn substitute_template_variable(directory: &Utf8Path, path: &mut String) { if let Some(stripped_path) = path.strip_prefix(TEMPLATE_VARIABLE) { if let Some(unleashed_path) = stripped_path.strip_prefix("/") { - *path = path_to_str(&directory.join(unleashed_path)).to_string(); + *path = directory.join(unleashed_path).into_string(); } else { - *path = path_to_str(&directory.join(stripped_path)).to_string(); + *path = directory.join(stripped_path).into_string(); } } } From 84cd47b0ea588616fefde2393cad7217f8ca85f9 Mon Sep 17 00:00:00 2001 From: pshu Date: Sat, 30 May 2026 03:08:48 +0800 Subject: [PATCH 2/8] refactor: prefer into()/direct AsRef over explicit camino conversions Drop a redundant `as_std_path()` before `starts_with` (camino's `starts_with` takes `AsRef`) and use `.into()` for the unambiguous Utf8PathBuf -> PathBuf boundary conversions (error variants, parse args). No behavior change. --- src/cache.rs | 8 ++++---- src/lib.rs | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 21963f93..7a5b8ee7 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -378,8 +378,8 @@ impl CachedPathImpl { package_json_path.clone() }; match PackageJson::parse( - package_json_path.clone().into_std_path_buf(), - real_path.into_std_path_buf(), + package_json_path.clone().into(), + real_path.into(), package_json_string, ) { Ok(v) => Ok(Some(Arc::new(v))), @@ -396,7 +396,7 @@ impl CachedPathImpl { if let Some(err) = serde_err { Err(ResolveError::from_serde_json_error( - package_json_path.into_std_path_buf(), + package_json_path.into(), &err, Some(package_json_string), )) @@ -404,7 +404,7 @@ impl CachedPathImpl { let (line, column) = off_to_location(&package_json_string, parse_err.index()); Err(ResolveError::JSON(JSONError { - path: package_json_path.into_std_path_buf(), + path: package_json_path.into(), message: parse_err.error().to_string(), line, column, diff --git a/src/lib.rs b/src/lib.rs index 0fcfdd7b..97be5c36 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -301,7 +301,7 @@ impl ResolverGeneric { .await?; if let Some(package_json) = &package_json { // path must be inside the package. - debug_assert!(path.as_std_path().starts_with(package_json.directory())); + debug_assert!(path.starts_with(package_json.directory())); } Ok(Resolution { path, @@ -1337,7 +1337,7 @@ impl ResolverGeneric { } AliasValue::Ignore => { let path = cached_path.path().normalize_with(alias_key); - return Err(ResolveError::Ignored(path.into_std_path_buf())); + return Err(ResolveError::Ignored(path.into())); } } } @@ -1595,7 +1595,7 @@ impl ResolverGeneric { async move { if reference_tsconfig.path == current_path { return Err(ResolveError::TsconfigSelfReference( - reference_tsconfig.path.clone().into_std_path_buf(), + reference_tsconfig.path.clone().into(), )); } // Cut the cycle: if this reference is already part of the @@ -1807,7 +1807,7 @@ impl ResolverGeneric { without_dot = without_dot || !starts_with_dot_or_hash; if has_dot && without_dot { return Err(ResolveError::InvalidPackageConfig( - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } } @@ -1823,7 +1823,7 @@ impl ResolverGeneric { let fragment = ctx.fragment.clone().unwrap_or_default(); return Err(ResolveError::PackagePathNotExported( format!("./{}{query}{fragment}", subpath.trim_start_matches('.')), - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } // 1. Let mainExport be undefined. @@ -1896,7 +1896,7 @@ impl ResolverGeneric { // 4. Throw a Package Path Not Exported error. Err(ResolveError::PackagePathNotExported( subpath.to_string(), - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )) }; Box::pin(fut) @@ -2074,7 +2074,7 @@ impl ResolverGeneric { Cow::Owned(format!("{target}{pattern_match}")) } else { return Err(ResolveError::InvalidPackageConfigDirectory( - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } } else { @@ -2097,7 +2097,7 @@ impl ResolverGeneric { return Err(ResolveError::InvalidPackageTarget( target.to_string(), target_key.to_string(), - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } // 2. If patternMatch is a String, then @@ -2117,7 +2117,7 @@ impl ResolverGeneric { return Err(ResolveError::InvalidPackageTarget( target.to_string(), target_key.to_string(), - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } let resolved_target = package_url.normalize_with(target.as_ref()); @@ -2164,7 +2164,7 @@ impl ResolverGeneric { // Note: return PackagePathNotExported has the same effect as return because there are no matches. return Err(ResolveError::PackagePathNotExported( pattern_match.unwrap_or(".").to_string(), - package_url.join("package.json").into_std_path_buf(), + package_url.join("package.json").into(), )); } // 2. For each item targetValue in target, do From 9c5333079fba39c93bd9859f7d8d10ad3c8df45d Mon Sep 17 00:00:00 2001 From: pshu Date: Sat, 30 May 2026 03:19:32 +0800 Subject: [PATCH 3/8] fix(test): use Utf8PathBuf::as_str in windows-only test Resolution.path is now Utf8PathBuf, which has no `to_str`. The windows test reads the pub(crate) field directly; switch it to `as_str()`. This module is `#[cfg(windows)]`, so the breakage only surfaced on the Windows CI runner. --- src/tests/windows.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/tests/windows.rs b/src/tests/windows.rs index 8a7989e7..f7aa6b28 100644 --- a/src/tests/windows.rs +++ b/src/tests/windows.rs @@ -38,11 +38,7 @@ mod tests { .resolve_with_context(&file, &specifier, &mut ctx) .await .unwrap(); - let resolved_path_string = resolved - .path - .to_str() - .expect("path should be UTF-8") - .to_string(); + let resolved_path_string = resolved.path.as_str().to_string(); let actual_file_deps = ctx .file_dependencies .iter() From 36b59db428dfc5b87c4df1fa8d6fb722986d65ad Mon Sep 17 00:00:00 2001 From: pshu Date: Sat, 30 May 2026 11:22:09 +0800 Subject: [PATCH 4/8] perf(cache): compare cache keys by bytes instead of camino component-wise camino's `Utf8Path` `PartialEq` walks path components with no fast path, while std `Path` (and `&str`) equality is a single `memcmp` (std even comments its fast path is "for hashmap lookups"). The `DashSet` cache lookup is the hottest equality on the resolve path (~2.6M comparisons), so the component-wise compare regressed the benches by ~40% instructions. Compare `as_str()` bytes instead. Correct because cache entries are already byte-keyed via the precomputed `hash_path`, so a hash hit implies identical bytes; byte equality matches the existing hashing contract. --- src/cache.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 7a5b8ee7..09c5e4d0 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -114,7 +114,12 @@ impl Hash for CachedPath { impl PartialEq for CachedPath { fn eq(&self, other: &Self) -> bool { - self.0.path == other.0.path + // Compare raw bytes, not `Utf8Path == Utf8Path`. camino's `Utf8Path` + // equality walks path components (`self.components().eq(other.components())`) + // with no fast path, whereas `&str == &str` is a single `memcmp`. This is the + // hot cache-lookup equality and entries are already byte-keyed via the + // precomputed `hash_path`, so byte equality is both correct and faster. + self.0.path.as_str() == other.0.path.as_str() } } impl Eq for CachedPath {} @@ -451,7 +456,9 @@ impl Hash for dyn CacheKey + '_ { impl PartialEq for dyn CacheKey + '_ { fn eq(&self, other: &Self) -> bool { - self.tuple().1 == other.tuple().1 + // Byte comparison rather than camino's component-wise `Utf8Path` equality; + // see `CachedPath`'s `PartialEq` for the rationale. + self.tuple().1.as_str() == other.tuple().1.as_str() } } From 6f7005682d1c00a8b7d0b6410158b365443c9e14 Mon Sep 17 00:00:00 2001 From: pshu Date: Sat, 30 May 2026 14:16:09 +0800 Subject: [PATCH 5/8] fix(cache): use as_std_path() for cache equality so Windows dedup is correct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior `as_str()` byte comparison broke path dedup on Windows: there `hash_path` hashes component-wise (`Path::hash`), so two paths differing only by separator/case (`pack1/foo` vs `pack1\foo`) hash equal but compared unequal as raw strings — corrupting the cache. Compare via std `Path` (`as_std_path()`): component-correct on every platform (matches `hash_path`'s per-platform scheme, exactly like the pre-camino baseline) and still hits std's byte `memcmp` fast path, which is what removes the Linux benchmark regression. --- src/cache.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 09c5e4d0..3ca811de 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -114,12 +114,15 @@ impl Hash for CachedPath { impl PartialEq for CachedPath { fn eq(&self, other: &Self) -> bool { - // Compare raw bytes, not `Utf8Path == Utf8Path`. camino's `Utf8Path` - // equality walks path components (`self.components().eq(other.components())`) - // with no fast path, whereas `&str == &str` is a single `memcmp`. This is the - // hot cache-lookup equality and entries are already byte-keyed via the - // precomputed `hash_path`, so byte equality is both correct and faster. - self.0.path.as_str() == other.0.path.as_str() + // Compare through std `Path`, not camino's `Utf8Path`. std `Path`/`Components` + // equality has a raw-byte `memcmp` fast path (its own comment: "for hashmap + // lookups"), whereas camino's `Utf8Path` always walks components with no fast + // path — that regressed this hottest cache-lookup equality. `as_std_path()` + // also preserves the per-platform semantics that match `hash_path` + // (byte-wise on Unix; component-wise on Windows, so `pack1/foo` and + // `pack1\foo` stay the same entry). A plain `as_str()` byte compare would be + // inconsistent with the component-wise hash on Windows and break dedup there. + self.0.path.as_std_path() == other.0.path.as_std_path() } } impl Eq for CachedPath {} @@ -456,9 +459,9 @@ impl Hash for dyn CacheKey + '_ { impl PartialEq for dyn CacheKey + '_ { fn eq(&self, other: &Self) -> bool { - // Byte comparison rather than camino's component-wise `Utf8Path` equality; - // see `CachedPath`'s `PartialEq` for the rationale. - self.tuple().1.as_str() == other.tuple().1.as_str() + // std `Path` equality (memcmp fast path + per-platform semantics matching + // `hash_path`); see `CachedPath`'s `PartialEq` for the full rationale. + self.tuple().1.as_std_path() == other.tuple().1.as_std_path() } } From 114da369709954b5d330be122f930f3522265d76 Mon Sep 17 00:00:00 2001 From: pshu Date: Sun, 31 May 2026 08:20:46 +0800 Subject: [PATCH 6/8] test(camino): shrink test diff via public path() accessor and tighter conversions Route the Windows test through the public Resolution::path() accessor and build the expected pnp path in UTF-8 up front. Reverts incidental churn from the camino refactor; behavior is unchanged. --- src/tests/pnp.rs | 15 ++++++--------- src/tests/windows.rs | 7 +++++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index 0e1afcc6..3d55ee16 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -156,15 +156,12 @@ async fn pnp_resolve_description_file() { .to_str() .expect("path should be UTF-8") .to_string(), - Utf8Path::from_path( - &fixture - .join(".yarn/cache/preact-npm-10.25.4-2dd2c0aa44-33a009d614.zip/node_modules/preact") - .join("package.json"), - ) - .expect("path should be UTF-8") - .normalize() - .as_str() - .to_string() + Utf8Path::from_path(&fixture) + .expect("path should be UTF-8") + .join(".yarn/cache/preact-npm-10.25.4-2dd2c0aa44-33a009d614.zip/node_modules/preact") + .join("package.json") + .normalize() + .to_string() ); } diff --git a/src/tests/windows.rs b/src/tests/windows.rs index f7aa6b28..492545de 100644 --- a/src/tests/windows.rs +++ b/src/tests/windows.rs @@ -38,7 +38,11 @@ mod tests { .resolve_with_context(&file, &specifier, &mut ctx) .await .unwrap(); - let resolved_path_string = resolved.path.as_str().to_string(); + let resolved_path_string = resolved + .path() + .to_str() + .expect("path should be UTF-8") + .to_string(); let actual_file_deps = ctx .file_dependencies .iter() @@ -53,7 +57,6 @@ mod tests { Utf8Path::from_path(p.as_ref()) .expect("path should be UTF-8") .normalize() - .as_str() .to_string() } } From 936dfb1a38db0f48163ce8c38008eb75b0eb9d6f Mon Sep 17 00:00:00 2001 From: pshu Date: Sun, 31 May 2026 11:34:30 +0800 Subject: [PATCH 7/8] refactor(test): use .into() for camino path literals Lean on camino's From impls (From<&str> for &Utf8Path, From<&str> for Utf8PathBuf) to replace explicit Utf8Path::new / Utf8PathBuf::from where the target type is inferable from context. --- src/resolution.rs | 2 +- src/tests/exports_field.rs | 2 +- src/tests/imports_field.rs | 2 +- src/tests/tsconfig_paths.rs | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/resolution.rs b/src/resolution.rs index a6e949d7..0acab18f 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -82,7 +82,7 @@ impl Resolution { #[tokio::test] async fn test() { let resolution = Resolution { - path: Utf8PathBuf::from("foo"), + path: "foo".into(), query: Some("?query".to_string()), fragment: Some("#fragment".to_string()), package_json: None, diff --git a/src/tests/exports_field.rs b/src/tests/exports_field.rs index e63f4d64..2267f0c0 100644 --- a/src/tests/exports_field.rs +++ b/src/tests/exports_field.rs @@ -2603,7 +2603,7 @@ async fn test_cases() { ..ResolveOptions::default() }) .package_exports_resolve( - Utf8Path::new(""), + "".into(), case.request, &case.exports_field, &mut Ctx::default(), diff --git a/src/tests/imports_field.rs b/src/tests/imports_field.rs index 135c5294..cfafde66 100644 --- a/src/tests/imports_field.rs +++ b/src/tests/imports_field.rs @@ -1307,7 +1307,7 @@ async fn test_cases() { .package_imports_exports_resolve( case.request, case.imports_field.as_object().unwrap(), - Utf8Path::new(""), + "".into(), true, &case .condition_names diff --git a/src/tests/tsconfig_paths.rs b/src/tests/tsconfig_paths.rs index 8237b5a5..9f071c61 100644 --- a/src/tests/tsconfig_paths.rs +++ b/src/tests/tsconfig_paths.rs @@ -2,7 +2,7 @@ //! //! Fixtures copied from . -use camino::{Utf8Path, Utf8PathBuf}; +use camino::Utf8PathBuf; use crate::{ JSONError, ResolveError, ResolveOptions, Resolver, TsConfig, TsconfigOptions, TsconfigReferences, @@ -138,7 +138,7 @@ async fn empty() { // #[tokio::test] async fn test_paths() { - let path = Utf8Path::new("/foo/tsconfig.json"); + let path = "/foo/tsconfig.json".into(); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "paths": { @@ -178,7 +178,7 @@ async fn test_paths() { // #[tokio::test] async fn test_base_url() { - let path = Utf8Path::new("/foo/tsconfig.json"); + let path = "/foo/tsconfig.json".into(); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "baseUrl": "./src" @@ -206,7 +206,7 @@ async fn test_base_url() { // #[tokio::test] async fn test_paths_and_base_url() { - let path = Utf8Path::new("/foo/tsconfig.json"); + let path = "/foo/tsconfig.json".into(); let mut tsconfig_json = serde_json::json!({ "compilerOptions": { "baseUrl": "./src", From 91ec1542fba1621cca1c4b1fb7d5e28e5afc5282 Mon Sep 17 00:00:00 2001 From: pshu Date: Sun, 31 May 2026 16:09:52 +0800 Subject: [PATCH 8/8] refactor(test): use camino cross-type PartialEq instead of mapping to Utf8PathBuf Utf8PathBuf implements PartialEq, so the resolve_path_alias assertions compare Vec against the original Vec directly, reverting those lines to match main. --- src/tests/tsconfig_paths.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/tests/tsconfig_paths.rs b/src/tests/tsconfig_paths.rs index 9f071c61..3ed12521 100644 --- a/src/tests/tsconfig_paths.rs +++ b/src/tests/tsconfig_paths.rs @@ -2,7 +2,7 @@ //! //! Fixtures copied from . -use camino::Utf8PathBuf; +use std::path::PathBuf; use crate::{ JSONError, ResolveError, ResolveOptions, Resolver, TsConfig, TsconfigOptions, TsconfigReferences, @@ -167,10 +167,7 @@ async fn test_paths() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected - .into_iter() - .map(Utf8PathBuf::from) - .collect::>(); + let expected = expected.into_iter().map(PathBuf::from).collect::>(); assert_eq!(paths, expected, "{specifier}"); } } @@ -195,10 +192,7 @@ async fn test_base_url() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected - .into_iter() - .map(Utf8PathBuf::from) - .collect::>(); + let expected = expected.into_iter().map(PathBuf::from).collect::>(); assert_eq!(paths, expected, "{specifier}"); } } @@ -241,10 +235,7 @@ async fn test_paths_and_base_url() { for (specifier, expected) in data { let paths = tsconfig.resolve_path_alias(specifier); - let expected = expected - .into_iter() - .map(Utf8PathBuf::from) - .collect::>(); + let expected = expected.into_iter().map(PathBuf::from).collect::>(); assert_eq!(paths, expected, "{specifier}"); } }