feat: add yarn_pnp logic to FileSystem (#589)

Running fs operations through yarn pnp incurs significant performance
cost, caused by running `VPath::from(path)` on every path.

Feature gate this through a runtime flag,
because a bundler can enable `yarn_pnp` feature but run the resolver
without presence of `yarn`.

Author: Boshen

.github/workflows/ci.yml

@@ -44,6 +44,7 @@ jobs:
       - uses: ./.github/actions/pnpm
       - run: cargo check --all-features --locked
       - run: cargo test --doc
+      - run: cargo test # without features
       - run: cargo test --all-features
       - run: pnpm run build:debug && pnpm run test
         if: ${{ matrix.os  != 'windows-latest' }}

justfile

@@ -54,6 +54,7 @@ check:
 
 # Run all the tests
 test:
+  cargo test
   cargo test --all-features
   node --run build
   node --run test

src/file_system.rs

@@ -9,6 +9,12 @@ use pnp::fs::{LruZipCache, VPath, VPathInfo, ZipCache};
 
 /// File System abstraction used for `ResolverGeneric`
 pub trait FileSystem: Send + Sync {
+    #[cfg(feature = "yarn_pnp")]
+    fn new(yarn_pnp: bool) -> Self;
+
+    #[cfg(not(feature = "yarn_pnp"))]
+    fn new() -> Self;
+
     /// See [std::fs::read_to_string]
     ///
     /// # Errors
@@ -95,25 +101,13 @@ impl From<fs::Metadata> for FileMetadata {
     }
 }
 
-/// Operating System
-#[cfg(feature = "yarn_pnp")]
-pub struct FileSystemOs {
-    pnp_lru: LruZipCache<Vec<u8>>,
-}
-
 #[cfg(not(feature = "yarn_pnp"))]
 pub struct FileSystemOs;
 
-impl Default for FileSystemOs {
-    fn default() -> Self {
-        cfg_if! {
-            if #[cfg(feature = "yarn_pnp")] {
-                Self { pnp_lru: LruZipCache::new(50, pnp::fs::open_zip_via_read_p) }
-            } else {
-                Self
-            }
-        }
-    }
+#[cfg(feature = "yarn_pnp")]
+pub struct FileSystemOs {
+    pnp_lru: LruZipCache<Vec<u8>>,
+    yarn_pnp: bool,
 }
 
 impl FileSystemOs {
@@ -171,38 +165,51 @@ impl FileSystemOs {
 }
 
 impl FileSystem for FileSystemOs {
+    #[cfg(feature = "yarn_pnp")]
+    fn new(yarn_pnp: bool) -> Self {
+        Self { pnp_lru: LruZipCache::new(50, pnp::fs::open_zip_via_read_p), yarn_pnp }
+    }
+
+    #[cfg(not(feature = "yarn_pnp"))]
+    fn new() -> Self {
+        Self
+    }
+
     fn read_to_string(&self, path: &Path) -> io::Result<String> {
         cfg_if! {
             if #[cfg(feature = "yarn_pnp")] {
-                match VPath::from(path)? {
-                    VPath::Zip(info) => {
-                        self.pnp_lru.read_to_string(info.physical_base_path(), info.zip_path)
+                if self.yarn_pnp {
+                    return match VPath::from(path)? {
+                        VPath::Zip(info) => {
+                            self.pnp_lru.read_to_string(info.physical_base_path(), info.zip_path)
+                        }
+                        VPath::Virtual(info) => Self::read_to_string(&info.physical_base_path()),
+                        VPath::Native(path) => Self::read_to_string(&path),
                     }
-                    VPath::Virtual(info) => Self::read_to_string(&info.physical_base_path()),
-                    VPath::Native(path) => Self::read_to_string(&path),
                 }
-            } else {
-                Self::read_to_string(path)
             }
         }
+        Self::read_to_string(path)
     }
 
     fn metadata(&self, path: &Path) -> io::Result<FileMetadata> {
         cfg_if! {
             if #[cfg(feature = "yarn_pnp")] {
-                match VPath::from(path)? {
-                    VPath::Zip(info) => self
-                        .pnp_lru
-                        .file_type(info.physical_base_path(), info.zip_path)
-                        .map(FileMetadata::from),
-                    VPath::Virtual(info) => {
-                        Self::metadata(&info.physical_base_path())
+                if self.yarn_pnp {
+                    return match VPath::from(path)? {
+                        VPath::Zip(info) => self
+                            .pnp_lru
+                            .file_type(info.physical_base_path(), info.zip_path)
+                            .map(FileMetadata::from),
+                        VPath::Virtual(info) => {
+                            Self::metadata(&info.physical_base_path())
+                        }
+                        VPath::Native(path) => Self::metadata(&path),
                     }
-                    VPath::Native(path) => Self::metadata(&path),
                 }
-            } else {
-                Self::metadata(path)}
+            }
         }
+        Self::metadata(path)
     }
 
     fn symlink_metadata(&self, path: &Path) -> io::Result<FileMetadata> {
@@ -212,15 +219,16 @@ impl FileSystem for FileSystemOs {
     fn read_link(&self, path: &Path) -> io::Result<PathBuf> {
         cfg_if! {
             if #[cfg(feature = "yarn_pnp")] {
-                match VPath::from(path)? {
-                    VPath::Zip(info) => Self::read_link(&info.physical_base_path().join(info.zip_path)),
-                    VPath::Virtual(info) => Self::read_link(&info.physical_base_path()),
-                    VPath::Native(path) => Self::read_link(&path),
+                if self.yarn_pnp {
+                    return match VPath::from(path)? {
+                        VPath::Zip(info) => Self::read_link(&info.physical_base_path().join(info.zip_path)),
+                        VPath::Virtual(info) => Self::read_link(&info.physical_base_path()),
+                        VPath::Native(path) => Self::read_link(&path),
+                    }
                 }
-            } else {
-                Self::read_link(path)
             }
         }
+        Self::read_link(path)
     }
 }
 

src/lib.rs

@@ -125,16 +125,24 @@ impl<Fs> fmt::Debug for ResolverGeneric<Fs> {
     }
 }
 
-impl<Fs: FileSystem + Default> Default for ResolverGeneric<Fs> {
+impl<Fs: FileSystem> Default for ResolverGeneric<Fs> {
     fn default() -> Self {
         Self::new(ResolveOptions::default())
     }
 }
 
-impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
+impl<Fs: FileSystem> ResolverGeneric<Fs> {
     #[must_use]
     pub fn new(options: ResolveOptions) -> Self {
-        Self { options: options.sanitize(), cache: Arc::new(Cache::new(Fs::default())) }
+        cfg_if::cfg_if! {
+            if #[cfg(feature = "yarn_pnp")] {
+                let fs = Fs::new(options.yarn_pnp);
+            } else {
+                let fs = Fs::new();
+            }
+        }
+        let cache = Arc::new(Cache::new(fs));
+        Self { options: options.sanitize(), cache }
     }
 }
 

src/tests/memory_fs.rs

@@ -42,6 +42,16 @@ impl MemoryFS {
 }
 
 impl FileSystem for MemoryFS {
+    #[cfg(not(feature = "yarn_pnp"))]
+    fn new() -> Self {
+        Self::default()
+    }
+
+    #[cfg(feature = "yarn_pnp")]
+    fn new(_yarn_pnp: bool) -> Self {
+        Self::default()
+    }
+
     fn read_to_string(&self, path: &Path) -> io::Result<String> {
         use vfs::FileSystem;
         let mut file = self