diff --git a/Cargo.lock b/Cargo.lock index 85a1b66d1..27a9f7fb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1490,6 +1490,7 @@ version = "0.0.0" dependencies = [ "criterion", "dashmap", + "dunce", "jemallocator", "mimalloc", "nodejs-resolver", diff --git a/crates/oxc_resolver/Cargo.toml b/crates/oxc_resolver/Cargo.toml index 7f28075ca..8d509b692 100644 --- a/crates/oxc_resolver/Cargo.toml +++ b/crates/oxc_resolver/Cargo.toml @@ -15,6 +15,7 @@ dashmap = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } rustc-hash = { workspace = true } +dunce = "1.0.4" [dev-dependencies] static_assertions = { workspace = true } diff --git a/crates/oxc_resolver/README.md b/crates/oxc_resolver/README.md index 57b5c907e..45a24ac96 100644 --- a/crates/oxc_resolver/README.md +++ b/crates/oxc_resolver/README.md @@ -32,7 +32,7 @@ | ✅ | preferAbsolute | false | Prefer to resolve server-relative urls as absolute paths before falling back to resolve in roots | | | restrictions | [] | A list of resolve restrictions | | ✅ | roots | [] | A list of root paths | -| | symlinks | true | Whether to resolve symlinks to their symlinked location | +| ✅ | symlinks | true | Whether to resolve symlinks to their symlinked location | | | unsafeCache | false | Use this cache object to unsafely cache the successful requests ## Test @@ -64,6 +64,6 @@ Tests ported from [enhanced-resolve](https://github.com/webpack/enhanced-resolve - [x] roots.test.js (need to add resolveToContext) - [x] scoped-packages.test.js - [x] simple.test.js -- [ ] symlink.test.js +- [x] symlink.test.js - [ ] unsafe-cache.test.js - [ ] yield.test.js diff --git a/crates/oxc_resolver/src/cache.rs b/crates/oxc_resolver/src/cache.rs index a2d4977a2..e9a6de4f7 100644 --- a/crates/oxc_resolver/src/cache.rs +++ b/crates/oxc_resolver/src/cache.rs @@ -1,4 +1,8 @@ -use std::{hash::BuildHasherDefault, path::Path, sync::Arc}; +use std::{ + hash::BuildHasherDefault, + path::{Path, PathBuf}, + sync::Arc, +}; use dashmap::DashMap; use rustc_hash::FxHasher; @@ -36,7 +40,7 @@ impl Cache { if let Some(result) = self.cache.get(path) { return *result; } - let file_metadata = self.fs.symlink_metadata(path).ok(); + let file_metadata = self.fs.metadata(path).ok(); self.cache.insert(path.to_path_buf().into_boxed_path(), file_metadata); file_metadata } @@ -45,6 +49,10 @@ impl Cache { self.metadata_cached(path).is_some_and(|m| m.is_file) } + pub fn canonicalize(&self, path: PathBuf) -> PathBuf { + self.fs.canonicalize(&path).unwrap_or(path) + } + /// # Errors /// /// * [ResolveError::JSONError] diff --git a/crates/oxc_resolver/src/file_system.rs b/crates/oxc_resolver/src/file_system.rs index 634008f54..bddcdd134 100644 --- a/crates/oxc_resolver/src/file_system.rs +++ b/crates/oxc_resolver/src/file_system.rs @@ -1,11 +1,18 @@ -use std::{fs, io, path::Path}; +use std::{ + fs, io, + path::{Path, PathBuf}, +}; pub trait FileSystem: Default + Send + Sync { + /// See [std::fs::read_to_string] + /// /// # Errors /// /// * Any [io::Error] fn read_to_string>(&self, path: P) -> io::Result; + /// See [std::fs::metadata] + /// /// # Errors /// /// This function will return an error in the following situations, but is not @@ -13,7 +20,18 @@ pub trait FileSystem: Default + Send + Sync { /// /// * The user lacks permissions to perform `metadata` call on `path`. /// * `path` does not exist. - fn symlink_metadata>(&self, path: P) -> io::Result; + fn metadata>(&self, path: P) -> io::Result; + + /// See [std::fs::canonicalize] + /// + /// # Errors + /// + /// This function will return an error in the following situations, but is not + /// limited to just these cases: + /// + /// * `path` does not exist. + /// * A non-final component in path is not a directory. + fn canonicalize>(&self, path: P) -> io::Result; } #[derive(Debug, Clone, Copy)] @@ -36,7 +54,11 @@ impl FileSystem for FileSystemOs { fs::read_to_string(path) } - fn symlink_metadata>(&self, path: P) -> io::Result { - fs::symlink_metadata(path).map(|metadata| FileMetadata { is_file: metadata.is_file() }) + fn metadata>(&self, path: P) -> io::Result { + fs::metadata(path).map(|metadata| FileMetadata { is_file: metadata.is_file() }) + } + + fn canonicalize>(&self, path: P) -> io::Result { + dunce::canonicalize(path) } } diff --git a/crates/oxc_resolver/src/lib.rs b/crates/oxc_resolver/src/lib.rs index 32fb0bc10..8fdbba5fc 100644 --- a/crates/oxc_resolver/src/lib.rs +++ b/crates/oxc_resolver/src/lib.rs @@ -89,6 +89,7 @@ impl ResolverGeneric { result? } }; + let path = self.load_symlink(path); Ok(Resolution { path, query: request.query.map(ToString::to_string), @@ -182,6 +183,10 @@ impl ResolverGeneric { Ok(None) } + fn load_symlink(&self, path: PathBuf) -> PathBuf { + if self.options.symlinks { self.cache.canonicalize(path) } else { path } + } + #[allow(clippy::unnecessary_wraps)] fn load_index(&self, path: &Path) -> ResolveState { for main_field in &self.options.main_files { diff --git a/crates/oxc_resolver/src/options.rs b/crates/oxc_resolver/src/options.rs index fde1ecd40..0391c5ab7 100644 --- a/crates/oxc_resolver/src/options.rs +++ b/crates/oxc_resolver/src/options.rs @@ -76,6 +76,13 @@ pub struct ResolveOptions { /// /// Default `[]` pub roots: Vec, + + /// Whether to resolve symlinks to their symlinked location. + /// When enabled, symlinked resources are resolved to their real path, not their symlinked location. + /// Note that this may cause module resolution to fail when using tools that symlink packages (like npm link). + /// + /// Default `true` + pub symlinks: bool, } impl Default for ResolveOptions { @@ -93,6 +100,7 @@ impl Default for ResolveOptions { prefer_relative: false, prefer_absolute: false, roots: vec![], + symlinks: true, } } } diff --git a/crates/oxc_resolver/tests/enhanced_resolve/test/.gitignore b/crates/oxc_resolver/tests/enhanced_resolve/test/.gitignore new file mode 100644 index 000000000..bbbec5f44 --- /dev/null +++ b/crates/oxc_resolver/tests/enhanced_resolve/test/.gitignore @@ -0,0 +1,2 @@ +# created by symlink.rs +/temp diff --git a/crates/oxc_resolver/tests/enhanced_resolve/test/mod.rs b/crates/oxc_resolver/tests/enhanced_resolve/test/mod.rs index baee4b16d..e183393ce 100644 --- a/crates/oxc_resolver/tests/enhanced_resolve/test/mod.rs +++ b/crates/oxc_resolver/tests/enhanced_resolve/test/mod.rs @@ -8,6 +8,7 @@ mod resolve; mod roots; mod scoped_packages; mod simple; +mod symlink; use std::{env, path::PathBuf}; diff --git a/crates/oxc_resolver/tests/enhanced_resolve/test/symlink.rs b/crates/oxc_resolver/tests/enhanced_resolve/test/symlink.rs new file mode 100644 index 000000000..75f531fe1 --- /dev/null +++ b/crates/oxc_resolver/tests/enhanced_resolve/test/symlink.rs @@ -0,0 +1,126 @@ +use std::{env, fs, io, path::Path}; + +use oxc_resolver::{Resolution, ResolveOptions, Resolver}; + +#[derive(Debug, Clone, Copy)] +enum FileType { + File, + Dir, +} + +#[allow(unused_variables)] +fn symlink, Q: AsRef>( + original: P, + link: Q, + file_type: FileType, +) -> io::Result<()> { + #[cfg(target_family = "unix")] + { + std::os::unix::fs::symlink(original, link) + } + + #[cfg(target_family = "windows")] + match file_type { + FileType::File => std::os::windows::fs::symlink_file(original, link), + FileType::Dir => std::os::windows::fs::symlink_dir(original, link), + } +} + +fn init(dirname: &Path, temp_path: &Path) -> io::Result<()> { + if temp_path.exists() { + _ = fs::remove_dir_all(temp_path); + } + fs::create_dir(temp_path)?; + symlink(dirname.join("../lib/index.js"), temp_path.join("test"), FileType::File)?; + symlink(dirname.join("../lib"), temp_path.join("test2"), FileType::Dir)?; + fs::remove_file(temp_path.join("test"))?; + fs::remove_file(temp_path.join("test2"))?; + fs::remove_dir(temp_path) +} + +fn create_symlinks(dirname: &Path, temp_path: &Path) -> io::Result<()> { + fs::create_dir(temp_path).unwrap(); + symlink( + dirname.join("../lib/index.js").canonicalize().unwrap(), + temp_path.join("index.js"), + FileType::File, + )?; + symlink(dirname.join("../lib").canonicalize().unwrap(), temp_path.join("lib"), FileType::Dir)?; + symlink(dirname.join("..").canonicalize().unwrap(), temp_path.join("this"), FileType::Dir)?; + symlink(temp_path.join("this"), temp_path.join("that"), FileType::Dir)?; + symlink(Path::new("../../lib/index.js"), temp_path.join("node.relative.js"), FileType::File)?; + symlink( + Path::new("./node.relative.js"), + temp_path.join("node.relative.sym.js"), + FileType::File, + )?; + Ok(()) +} + +fn cleanup_symlinks(temp_path: &Path) { + _ = fs::remove_dir_all(temp_path); +} + +#[test] +fn test() -> io::Result<()> { + let root = env::current_dir().unwrap().join("tests/enhanced_resolve"); + let dirname = root.join("test"); + let temp_path = dirname.join("temp"); + if !temp_path.exists() { + let is_admin = init(&dirname, &temp_path).is_ok(); + if !is_admin { + return Ok(()); + } + if let Err(err) = create_symlinks(&dirname, &temp_path) { + cleanup_symlinks(&temp_path); + return Err(err); + } + } + + let resolver_without_symlinks = + Resolver::new(ResolveOptions { symlinks: false, ..ResolveOptions::default() }); + let resolver_with_symlinks = Resolver::default(); + + #[rustfmt::skip] + let pass = [ + ("with a symlink to a file", temp_path.clone(), "./index.js"), + ("with a relative symlink to a file", temp_path.clone(), "./node.relative.js"), + ("with a relative symlink to a symlink to a file", temp_path.clone(), "./node.relative.sym.js"), + ("with a symlink to a directory 1", temp_path.clone(), "./lib/index.js"), + ("with a symlink to a directory 2", temp_path.clone(), "./this/lib/index.js"), + ("with multiple symlinks in the path 1", temp_path.clone(), "./this/test/temp/index.js"), + ("with multiple symlinks in the path 2", temp_path.clone(), "./this/test/temp/lib/index.js"), + ("with multiple symlinks in the path 3", temp_path.clone(), "./this/test/temp/this/lib/index.js"), + ("with a symlink to a directory 2 (chained)", temp_path.clone(), "./that/lib/index.js"), + ("with multiple symlinks in the path 1 (chained)", temp_path.clone(), "./that/test/temp/index.js"), + ("with multiple symlinks in the path 2 (chained)", temp_path.clone(), "./that/test/temp/lib/index.js"), + ("with multiple symlinks in the path 3 (chained)", temp_path.clone(), "./that/test/temp/that/lib/index.js"), + ("with symlinked directory as context 1", temp_path.join( "lib"), "./index.js"), + ("with symlinked directory as context 2", temp_path.join( "this"), "./lib/index.js"), + ("with symlinked directory as context and in path", temp_path.join( "this"), "./test/temp/lib/index.js"), + ("with symlinked directory in context path", temp_path.join( "this/lib"), "./index.js"), + ("with symlinked directory in context path and symlinked file", temp_path.join( "this/test"), "./temp/index.js"), + ("with symlinked directory in context path and symlinked directory", temp_path.join( "this/test"), "./temp/lib/index.js"), + ("with symlinked directory as context 2 (chained)", temp_path.join( "that"), "./lib/index.js"), + ("with symlinked directory as context and in path (chained)", temp_path.join( "that"), "./test/temp/lib/index.js"), + ("with symlinked directory in context path (chained)", temp_path.join( "that/lib"), "./index.js"), + ("with symlinked directory in context path and symlinked file (chained)", temp_path.join( "that/test"), "./temp/index.js"), + ("with symlinked directory in context path and symlinked directory (chained)", temp_path.join( "that/test"), "./temp/lib/index.js") + ]; + + for (comment, path, request) in pass { + let filename = resolver_with_symlinks.resolve(&path, request).map_or_else( + |err| { + panic!("{err:?} {comment} {path:?} {request}"); + }, + Resolution::full_path, + ); + assert_eq!(filename, root.join("lib/index.js")); + + let resolved_path = + resolver_without_symlinks.resolve(&path, request).map(Resolution::full_path); + assert_eq!(resolved_path, Ok(path.join(request))); + } + + Ok(()) +} diff --git a/crates/oxc_resolver/tests/memory_fs.rs b/crates/oxc_resolver/tests/memory_fs.rs index 8b5b917ee..8fe6f9ac4 100644 --- a/crates/oxc_resolver/tests/memory_fs.rs +++ b/crates/oxc_resolver/tests/memory_fs.rs @@ -1,4 +1,7 @@ -use std::{io, path::Path}; +use std::{ + io, + path::{Path, PathBuf}, +}; use oxc_resolver::{FileMetadata, FileSystem}; @@ -48,7 +51,7 @@ impl FileSystem for MemoryFS { Ok(buffer) } - fn symlink_metadata>(&self, path: P) -> io::Result { + fn metadata>(&self, path: P) -> io::Result { use vfs::FileSystem; let metadata = self .fs @@ -57,4 +60,8 @@ impl FileSystem for MemoryFS { let is_file = metadata.file_type == vfs::VfsFileType::File; Ok(FileMetadata::new(is_file)) } + + fn canonicalize>(&self, path: P) -> io::Result { + Ok(path.as_ref().to_path_buf()) + } }