diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5ec3a45..6c916698 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,13 +78,13 @@ jobs: - name: corepack run: corepack enable - + - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - + - name: Install dependencies run: pnpm install --frozen-lockfile @@ -211,6 +211,8 @@ jobs: - os: ubuntu-latest - os: macos-14 runs-on: ${{ matrix.os }} + env: + RUST_BACKTRACE: 1 steps: - uses: actions/checkout@v4 - uses: ./.github/actions/pnpm diff --git a/Cargo.lock b/Cargo.lock index f751eac0..52c41963 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -426,6 +426,12 @@ dependencies = [ "libloading", ] +[[package]] +name = "normalize-path" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5438dd2b2ff4c6df6e1ce22d825ed2fa93ee2922235cc45186991717f0a892d" + [[package]] name = "num-traits" version = "0.2.17" @@ -467,6 +473,7 @@ dependencies = [ "dunce", "indexmap", "json-strip-comments", + "normalize-path", "once_cell", "rayon", "rustc-hash", diff --git a/Cargo.toml b/Cargo.toml index c1908f8c..e33f61d5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,9 +14,7 @@ rust-version = "1.60" include = ["/src", "/examples", "/benches"] [workspace] -members = [ - "napi", -] +members = ["napi"] [lib] doctest = false @@ -89,9 +87,10 @@ json-strip-comments = { version = "1.0.2" } codspeed-criterion-compat = { version = "2.3.3", default-features = false, optional = true } [dev-dependencies] -vfs = "0.10.0" # for testing with in memory file system -rayon = { version = "1.8.1" } -criterion = { version = "0.5.1", default-features = false } +vfs = "0.10.0" # for testing with in memory file system +rayon = { version = "1.8.1" } +criterion = { version = "0.5.1", default-features = false } +normalize-path = { version = "0.2.1" } [features] codspeed = ["codspeed-criterion-compat"] diff --git a/src/lib.rs b/src/lib.rs index d800a477..9299c091 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -244,7 +244,7 @@ impl ResolverGeneric { match specifier.as_bytes()[0] { // 3. If X begins with './' or '/' or '../' - b'/' => self.require_absolute(cached_path, specifier, ctx), + b'/' | b'\\' => self.require_absolute(cached_path, specifier, ctx), // 3. If X begins with './' or '/' or '../' b'.' => self.require_relative(cached_path, specifier, ctx), // 4. If X begins with '#' @@ -278,7 +278,7 @@ impl ResolverGeneric { specifier: &str, ctx: &mut Ctx, ) -> Result { - debug_assert!(specifier.starts_with('/')); + debug_assert!(specifier.starts_with(|c| c == '/' || c == '\\')); if !self.options.prefer_relative && self.options.prefer_absolute { if let Ok(path) = self.load_package_self_or_node_modules(cached_path, specifier, ctx) { return Ok(path); @@ -295,9 +295,11 @@ impl ResolverGeneric { } else { for root in &self.options.roots { let cached_path = self.cache.value(root); - if let Ok(path) = - self.require_relative(&cached_path, specifier.trim_start_matches('/'), ctx) - { + if let Ok(path) = self.require_relative( + &cached_path, + specifier.trim_start_matches(|c| c == '/' || c == '\\'), + ctx, + ) { return Ok(path); } } @@ -887,16 +889,25 @@ impl ResolverGeneric { && !request.strip_prefix(alias_value).is_some_and(|prefix| prefix.starts_with('/')) { let tail = &request[alias_key.len()..]; - // Must not append anything to alias_value if it is a file. - if !tail.is_empty() { - let alias_value_cached_path = self.cache.value(Path::new(alias_value)); + + let new_specifier = if tail.is_empty() { + Cow::Borrowed(alias_value) + } else { + let alias_value = Path::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_value); if alias_value_cached_path.is_file(&self.cache.fs, ctx) { return Ok(None); } - } - let new_specifier = format!("{alias_value}{tail}"); + + // Remove the leading slash so the final path is concatenated. + let tail = tail.trim_start_matches(|c| c == '/' || c == '\\'); + let normalized = alias_value.normalize_with(tail); + Cow::Owned(normalized.to_string_lossy().to_string()) + }; + ctx.with_fully_specified(false); - return match self.require(cached_path, &new_specifier, ctx) { + return match self.require(cached_path, new_specifier.as_ref(), ctx) { Err(ResolveError::NotFound(_)) => Ok(None), Ok(path) => return Ok(Some(path)), Err(err) => return Err(err), diff --git a/src/path.rs b/src/path.rs index 67bca5cd..354263f4 100644 --- a/src/path.rs +++ b/src/path.rs @@ -27,6 +27,7 @@ pub trait PathUtil { } impl PathUtil for Path { + // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L7 fn normalize(&self) -> PathBuf { let mut components = self.components().peekable(); let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek() { @@ -39,7 +40,7 @@ impl PathUtil for Path { for component in components { match component { - Component::Prefix(..) => unreachable!(), + Component::Prefix(..) => unreachable!("Path {:?}", self), Component::RootDir => { ret.push(component.as_os_str()); } @@ -56,6 +57,7 @@ impl PathUtil for Path { ret } + // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L37 fn normalize_with>(&self, subpath: B) -> PathBuf { let subpath = subpath.as_ref(); let mut components = subpath.components().peekable(); @@ -66,7 +68,9 @@ impl PathUtil for Path { let mut ret = self.to_path_buf(); for component in subpath.components() { match component { - Component::Prefix(..) | Component::RootDir => unreachable!(), + Component::Prefix(..) | Component::RootDir => { + unreachable!("Path {:?} Subpath {:?}", self, subpath) + } Component::CurDir => {} Component::ParentDir => { ret.pop(); diff --git a/src/tests/alias.rs b/src/tests/alias.rs index 16bf8656..1305caf3 100644 --- a/src/tests/alias.rs +++ b/src/tests/alias.rs @@ -1,6 +1,9 @@ //! -use crate::{AliasValue, Resolution, ResolveError, ResolveOptions, Resolver}; +use normalize_path::NormalizePath; +use std::path::Path; + +use crate::{AliasValue, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver}; #[test] #[cfg(not(target_os = "windows"))] // MemoryFS's path separator is always `/` so the test will not pass in windows. @@ -129,29 +132,33 @@ fn absolute_path() { assert_eq!(resolution, Err(ResolveError::Ignored(f.join("foo")))); } +fn check_slash(path: &Path) { + let s = path.to_string_lossy().to_string(); + #[cfg(target_os = "windows")] + { + assert!(!s.contains('/'), "{s}"); + assert!(s.contains('\\'), "{s}"); + } + #[cfg(not(target_os = "windows"))] + { + assert!(s.contains('/'), "{s}"); + assert!(!s.contains('\\'), "{s}"); + } +} + #[test] fn system_path() { let f = super::fixture(); let resolver = Resolver::new(ResolveOptions { alias: vec![( "@app".into(), - vec![AliasValue::Path(f.join("alias").to_str().unwrap().to_string())], + vec![AliasValue::Path(f.join("alias").to_string_lossy().to_string())], )], ..ResolveOptions::default() }); - let resolution = resolver.resolve(&f, "@app/files/a").map(Resolution::into_path_buf); - assert_eq!(resolution, Ok(f.join("alias/files/a.js"))); - let string = resolution.unwrap().to_string_lossy().to_string(); - #[cfg(target_os = "windows")] - { - assert!(!string.contains('/')); - assert!(string.contains('\\')); - } - #[cfg(not(target_os = "windows"))] - { - assert!(string.contains('/')); - assert!(!string.contains('\\')); - } + let path = resolver.resolve(&f, "@app/files/a").map(Resolution::into_path_buf).unwrap(); + assert_eq!(path, f.join("alias/files/a.js")); + check_slash(&path); } // Not part of enhanced-resolve @@ -168,3 +175,43 @@ fn infinite_recursion() { let resolution = resolver.resolve(f, "./a"); assert_eq!(resolution, Err(ResolveError::Recursion)); } + +#[test] +fn alias_is_full_path() { + let f = super::fixture(); + let dir = f.join("foo"); + let dir_str = dir.to_string_lossy().to_string(); + + let resolver = Resolver::new(ResolveOptions { + alias: vec![("@".into(), vec![AliasValue::Path(dir_str.clone())])], + ..ResolveOptions::default() + }); + + let mut ctx = ResolveContext::default(); + + let specifiers = [ + "@/index".to_string(), + // specifier has multiple `/` for reasons we'll never know + "@////index".to_string(), + // specifier is a full path + dir_str, + ]; + + for specifier in specifiers { + let resolution = resolver.resolve_with_context(&f, &specifier, &mut ctx); + assert_eq!(resolution.map(|r| r.full_path()), Ok(dir.join("index.js"))); + } + + for path in ctx.file_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); + check_slash(&path); + } + + for path in ctx.missing_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); + check_slash(&path); + if let Some(path) = path.parent() { + assert!(!path.is_file(), "{path:?} must not be a file"); + } + } +} diff --git a/src/tests/missing.rs b/src/tests/missing.rs index 19b1acff..4bf611ef 100644 --- a/src/tests/missing.rs +++ b/src/tests/missing.rs @@ -1,5 +1,7 @@ //! https://github.com/webpack/enhanced-resolve/blob/main/test/missing.test.js +use normalize_path::NormalizePath; + use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver}; #[test] @@ -52,10 +54,15 @@ fn test() { let mut ctx = ResolveContext::default(); let _ = resolver.resolve_with_context(&f, specifier, &mut ctx); - for dep in missing_dependencies { + for path in ctx.file_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); + } + + for path in missing_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); assert!( - ctx.missing_dependencies.contains(&dep), - "{specifier}: {dep:?} not in {:?}", + ctx.missing_dependencies.contains(&path), + "{specifier}: {path:?} not in {:?}", &ctx.missing_dependencies ); } @@ -86,8 +93,13 @@ fn alias_and_extensions() { let _ = resolver.resolve_with_context(&f, "@scope-js/package-name/dir/router", &mut ctx); let _ = resolver.resolve_with_context(&f, "react-dom/client", &mut ctx); - for dep in ctx.missing_dependencies { - if let Some(path) = dep.parent() { + for path in ctx.file_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); + } + + for path in ctx.missing_dependencies { + assert_eq!(path, path.normalize(), "{path:?}"); + if let Some(path) = path.parent() { assert!(!path.is_file(), "{path:?} must not be a file"); } } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index bf9738da..94e6bd86 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -29,7 +29,7 @@ pub fn fixture_root() -> PathBuf { } pub fn fixture() -> PathBuf { - fixture_root().join("enhanced_resolve/test/fixtures") + fixture_root().join("enhanced_resolve").join("test").join("fixtures") } #[test] diff --git a/src/tests/roots.rs b/src/tests/roots.rs index 8df5a8a5..460b2da9 100644 --- a/src/tests/roots.rs +++ b/src/tests/roots.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use crate::{AliasValue, ResolveError, ResolveOptions, Resolver}; fn dirname() -> PathBuf { - super::fixture_root().join("enhanced_resolve/test") + super::fixture_root().join("enhanced_resolve").join("test") } #[test]