feat(resolver): rework yarn manifest file look up (#586)

The current manifest file (`.pnp.cjs`) lookup technique does not work
when `enableGlobalCache` is set to `true`, which is the default behavior
for yarn.

Instead, we try and find the closest pnp maniest from cwd.

This will break the multi-project case mentioned in
https://yarnpkg.com/advanced/pnp-spec#find_pnp_manifest, but I think
this is an acceptable trade off.

For napi, I set `process.env.OXC_RESOLVER_YARN_PNP` in `napi/index.js`
so the Rust side can enable yarn pnp accordingly.

Author: Boshen

README.md

@@ -60,6 +60,13 @@ See https://stackblitz.com/edit/oxc-resolver for usage example.
 
 See [docs.rs/oxc_resolver](https://docs.rs/oxc_resolver/latest/oxc_resolver).
 
+### [Yarn Plug'n'Play](https://yarnpkg.com/features/pnp)
+
+- For node.js, yarn pnp should work without any configuration, given the following conditions:
+  - the program is called with the `yarn` command, where the value [`process.versions.pnp`](https://yarnpkg.com/advanced/pnpapi#processversionspnp) is set.
+  - `.pnp.cjs` manifest file exists in the closest directory, searched from the current working directory,
+  - no multi-project setup, per second bullet point in [FIND_PNP_MANIFEST](https://yarnpkg.com/advanced/pnp-spec#find_pnp_manifest)
+
 ## Terminology
 
 ### `directory`

napi/Cargo.toml

@@ -39,7 +39,7 @@ mimalloc-safe = { version = "0.1.52", optional = true, features = ["skip_collect
 napi-build = "2.2.1"
 
 [features]
-default = ["tracing-subscriber"]
+default = ["tracing-subscriber", "yarn_pnp"]
 allocator = ["dep:mimalloc-safe"]
 tracing-subscriber = ["dep:tracing-subscriber"]
 yarn_pnp = ["oxc_resolver/yarn_pnp"]

napi/index.d.ts

@@ -57,8 +57,6 @@ export interface NapiResolveOptions {
    * Default `None`
    */
   tsconfig?: TsconfigOptions
-  /** Enable Yarn Plug'n'Play */
-  yarnPnp?: boolean
   /**
    * Alias for [ResolveOptions::alias] and [ResolveOptions::fallback].
    *

napi/index.js

@@ -387,3 +387,7 @@ module.exports.ResolverFactory = nativeBinding.ResolverFactory
 module.exports.EnforceExtension = nativeBinding.EnforceExtension
 module.exports.ModuleType = nativeBinding.ModuleType
 module.exports.sync = nativeBinding.sync
+
+if (process.versions.pnp) {
+  process.env.OXC_RESOLVER_YARN_PNP = '1'
+}

napi/patch.mjs

@@ -15,4 +15,9 @@ if (!nativeBinding && globalThis.process?.versions?.["webcontainer"]) {
 }
 ` + s,
 );
+data = data + `
+if (process.versions.pnp) {
+  process.env.OXC_RESOLVER_YARN_PNP = '1'
+}
+`
 fs.writeFileSync(filename, data);

napi/src/lib.rs

@@ -151,8 +151,7 @@ impl ResolverFactory {
     #[napi]
     #[allow(clippy::should_implement_trait)]
     pub fn default() -> Self {
-        let default_options = ResolveOptions::default();
-        Self { resolver: Arc::new(Resolver::new(default_options)) }
+        Self { resolver: Arc::new(Resolver::new(ResolveOptions::default())) }
     }
 
     /// Clone the resolver using the same underlying cache.
@@ -190,6 +189,7 @@ impl ResolverFactory {
         let default = ResolveOptions::default();
         // merging options
         ResolveOptions {
+            cwd: None,
             tsconfig: op.tsconfig.map(|tsconfig| tsconfig.into()),
             alias: op
                 .alias
@@ -279,7 +279,7 @@ impl ResolverFactory {
                 .allow_package_exports_in_directory_resolve
                 .unwrap_or(default.allow_package_exports_in_directory_resolve),
             #[cfg(feature = "yarn_pnp")]
-            yarn_pnp: op.yarn_pnp.unwrap_or(default.yarn_pnp),
+            yarn_pnp: default.yarn_pnp,
         }
     }
 }

napi/src/options.rs

@@ -16,9 +16,6 @@ pub struct NapiResolveOptions {
     /// Default `None`
     pub tsconfig: Option<TsconfigOptions>,
 
-    /// Enable Yarn Plug'n'Play
-    pub yarn_pnp: Option<bool>,
-
     /// Alias for [ResolveOptions::alias] and [ResolveOptions::fallback].
     ///
     /// For the second value of the tuple, `None -> AliasValue::Ignore`, Some(String) ->

src/cache.rs

@@ -52,6 +52,9 @@ pub trait Cache: Sized {
         path: &Path,
         callback: F,
     ) -> Result<Arc<Self::Tc>, ResolveError>;
+
+    #[cfg(feature = "yarn_pnp")]
+    fn get_yarn_pnp_manifest(&self, cwd: Option<&Path>) -> Result<&pnp::Manifest, ResolveError>;
 }
 
 #[allow(clippy::missing_errors_doc)] // trait impls should be free to return any typesafe error

src/fs_cache.rs

@@ -41,6 +41,8 @@ pub struct FsCache<Fs> {
     pub(crate) fs: Fs,
     paths: HashSet<FsCachedPath, BuildHasherDefault<IdentityHasher>>,
     tsconfigs: HashMap<PathBuf, Arc<TsConfigSerde>, BuildHasherDefault<FxHasher>>,
+    #[cfg(feature = "yarn_pnp")]
+    yarn_pnp_manifest: OnceLock<pnp::Manifest>,
 }
 
 impl<Fs: FileSystem> Cache for FsCache<Fs> {
@@ -190,6 +192,18 @@ impl<Fs: FileSystem> Cache for FsCache<Fs> {
         tsconfigs.insert(path.to_path_buf(), Arc::clone(&tsconfig));
         Ok(tsconfig)
     }
+
+    #[cfg(feature = "yarn_pnp")]
+    fn get_yarn_pnp_manifest(&self, cwd: Option<&Path>) -> Result<&pnp::Manifest, ResolveError> {
+        self.yarn_pnp_manifest.get_or_try_init(|| {
+            // TODO: map to proper error
+            let cwd =
+                cwd.map_or_else(|| Cow::Owned(std::env::current_dir().unwrap()), Cow::Borrowed);
+            // TODO: map to proper error
+            let manifest = pnp::find_pnp_manifest(&cwd).unwrap().unwrap();
+            Ok(manifest)
+        })
+    }
 }
 
 impl<Fs: FileSystem> FsCache<Fs> {
@@ -204,6 +218,8 @@ impl<Fs: FileSystem> FsCache<Fs> {
                 .hasher(BuildHasherDefault::default())
                 .resize_mode(papaya::ResizeMode::Blocking)
                 .build(),
+            #[cfg(feature = "yarn_pnp")]
+            yarn_pnp_manifest: OnceLock::new(),
         }
     }
 

src/lib.rs

@@ -135,8 +135,6 @@ pub type Resolver = ResolverGeneric<FsCache<FileSystemOs>>;
 pub struct ResolverGeneric<C: Cache> {
     options: ResolveOptions,
     cache: Arc<C>,
-    #[cfg(feature = "yarn_pnp")]
-    pnp_cache: Arc<papaya::HashMap<C::Cp, Option<pnp::Manifest>>>,
 }
 
 impl<C: Cache> fmt::Debug for ResolverGeneric<C> {
@@ -154,34 +152,19 @@ impl<C: Cache + Default> Default for ResolverGeneric<C> {
 impl<C: Cache + Default> ResolverGeneric<C> {
     #[must_use]
     pub fn new(options: ResolveOptions) -> Self {
-        Self {
-            options: options.sanitize(),
-            cache: Arc::new(C::default()),
-            #[cfg(feature = "yarn_pnp")]
-            pnp_cache: Arc::new(papaya::HashMap::default()),
-        }
+        Self { options: options.sanitize(), cache: Arc::new(C::default()) }
     }
 }
 
 impl<C: Cache> ResolverGeneric<C> {
     pub fn new_with_cache(cache: Arc<C>, options: ResolveOptions) -> Self {
-        Self {
-            cache,
-            options: options.sanitize(),
-            #[cfg(feature = "yarn_pnp")]
-            pnp_cache: Arc::new(papaya::HashMap::default()),
-        }
+        Self { cache, options: options.sanitize() }
     }
 
     /// Clone the resolver using the same underlying cache.
     #[must_use]
     pub fn clone_with_options(&self, options: ResolveOptions) -> Self {
-        Self {
-            options: options.sanitize(),
-            cache: Arc::clone(&self.cache),
-            #[cfg(feature = "yarn_pnp")]
-            pnp_cache: Arc::clone(&self.pnp_cache),
-        }
+        Self { options: options.sanitize(), cache: Arc::clone(&self.cache) }
     }
 
     /// Returns the options.
@@ -883,92 +866,81 @@ impl<C: Cache> ResolverGeneric<C> {
         specifier: &str,
         ctx: &mut Ctx,
     ) -> Result<Option<C::Cp>, ResolveError> {
-        let pnp_cache = self.pnp_cache.pin();
-        let pnp_manifest = pnp_cache.get_or_insert_with(cached_path.clone(), || {
-            if let Some(path) = pnp::find_pnp_manifest(cached_path.path()).unwrap() {
-                return Some(path);
-            }
-            self.options.roots.iter().find_map(|root| pnp::find_pnp_manifest(root).unwrap())
-        });
+        let pnp_manifest = self.cache.get_yarn_pnp_manifest(self.options.cwd.as_deref())?;
 
-        if let Some(pnp_manifest) = pnp_manifest.as_ref() {
-            // "pnpapi" in a P'n'P builtin module
-            if specifier == "pnpapi" {
-                return Ok(Some(self.cache.value(pnp_manifest.manifest_path.as_path())));
-            }
+        // "pnpapi" in a P'n'P builtin module
+        if specifier == "pnpapi" {
+            return Ok(Some(self.cache.value(pnp_manifest.manifest_path.as_path())));
+        }
 
-            // `resolve_to_unqualified` requires a trailing slash
-            let mut path = cached_path.to_path_buf();
-            path.push("");
+        // `resolve_to_unqualified` requires a trailing slash
+        let mut path = cached_path.to_path_buf();
+        path.push("");
 
-            let resolution =
-                pnp::resolve_to_unqualified_via_manifest(pnp_manifest, specifier, path);
+        let resolution = pnp::resolve_to_unqualified_via_manifest(pnp_manifest, specifier, path);
 
-            match resolution {
-                Ok(pnp::Resolution::Resolved(path, subpath)) => {
-                    let cached_path = self.cache.value(&path);
-                    let cached_path_string = cached_path.path().to_string_lossy();
+        match resolution {
+            Ok(pnp::Resolution::Resolved(path, subpath)) => {
+                let cached_path = self.cache.value(&path);
+                let cached_path_string = cached_path.path().to_string_lossy();
 
-                    let export_resolution = self.load_package_self(&cached_path, specifier, ctx)?;
-                    // can be found in pnp cached folder
-                    if export_resolution.is_some() {
-                        return Ok(export_resolution);
-                    }
+                let export_resolution = self.load_package_self(&cached_path, specifier, ctx)?;
+                // can be found in pnp cached folder
+                if export_resolution.is_some() {
+                    return Ok(export_resolution);
+                }
 
-                    // symbol linked package doesn't have node_modules structure
-                    let pkg_name = cached_path_string.rsplit_once("node_modules/").map_or(
-                        "",
-                        // remove trailing slash
-                        |(_, last)| last.strip_suffix('/').unwrap_or(last),
-                    );
+                // symbol linked package doesn't have node_modules structure
+                let pkg_name = cached_path_string.rsplit_once("node_modules/").map_or(
+                    "",
+                    // remove trailing slash
+                    |(_, last)| last.strip_suffix('/').unwrap_or(last),
+                );
 
-                    let inner_request = if pkg_name.is_empty() {
-                        subpath.map_or_else(
-                            || ".".to_string(),
-                            |mut p| {
-                                p.insert_str(0, "./");
-                                p
-                            },
-                        )
+                let inner_request = if pkg_name.is_empty() {
+                    subpath.map_or_else(
+                        || ".".to_string(),
+                        |mut p| {
+                            p.insert_str(0, "./");
+                            p
+                        },
+                    )
+                } else {
+                    let (first, rest) = specifier.split_once('/').unwrap_or((specifier, ""));
+                    // the original `pkg_name` in cached path could be different with specifier
+                    // due to alias like `"custom-minimist": "npm:minimist@^1.2.8"`
+                    // in this case, `specifier` is `pkg_name`'s source of truth
+                    let pkg_name = if first.starts_with('@') {
+                        &format!("{first}/{}", rest.split_once('/').unwrap_or((rest, "")).0)
                     } else {
-                        let (first, rest) = specifier.split_once('/').unwrap_or((specifier, ""));
-                        // the original `pkg_name` in cached path could be different with specifier
-                        // due to alias like `"custom-minimist": "npm:minimist@^1.2.8"`
-                        // in this case, `specifier` is `pkg_name`'s source of truth
-                        let pkg_name = if first.starts_with('@') {
-                            &format!("{first}/{}", rest.split_once('/').unwrap_or((rest, "")).0)
-                        } else {
-                            first
-                        };
-                        let inner_specifier = specifier.strip_prefix(pkg_name).unwrap();
-                        String::from("./")
-                            + inner_specifier.strip_prefix("/").unwrap_or(inner_specifier)
+                        first
                     };
+                    let inner_specifier = specifier.strip_prefix(pkg_name).unwrap();
+                    String::from("./")
+                        + inner_specifier.strip_prefix("/").unwrap_or(inner_specifier)
+                };
 
-                    // it could be a directory with `package.json` that redirects to another file,
-                    // take `@atlaskit/pragmatic-drag-and-drop` for example, as described at import-js/eslint-import-resolver-typescript#409
-                    if let Ok(Some(result)) = self.load_as_directory(
-                        &self.cache.value(&path.join(inner_request.clone()).normalize()),
-                        ctx,
-                    ) {
-                        return Ok(Some(result));
-                    }
+                // it could be a directory with `package.json` that redirects to another file,
+                // take `@atlaskit/pragmatic-drag-and-drop` for example, as described at import-js/eslint-import-resolver-typescript#409
+                if let Ok(Some(result)) = self.load_as_directory(
+                    &self.cache.value(&path.join(inner_request.clone()).normalize()),
+                    ctx,
+                ) {
+                    return Ok(Some(result));
+                }
 
-                    let inner_resolver = self.clone_with_options(self.options().clone());
+                let inner_resolver = self.clone_with_options(self.options().clone());
 
-                    // try as file or directory `path` in the pnp folder
-                    let Ok(inner_resolution) = inner_resolver.resolve(&path, &inner_request) else {
-                        return Err(ResolveError::NotFound(specifier.to_string()));
-                    };
-
-                    Ok(Some(self.cache.value(inner_resolution.path())))
-                }
+                // try as file or directory `path` in the pnp folder
+                let Ok(inner_resolution) = inner_resolver.resolve(&path, &inner_request) else {
+                    return Err(ResolveError::NotFound(specifier.to_string()));
+                };
 
-                Ok(pnp::Resolution::Skipped) => Ok(None),
-                Err(_) => Err(ResolveError::NotFound(specifier.to_string())),
+                Ok(Some(self.cache.value(inner_resolution.path())))
             }
-        } else {
-            Ok(None)
+
+            Ok(pnp::Resolution::Skipped) => Ok(None),
+            Err(_) => Err(ResolveError::NotFound(specifier.to_string())),
         }
     }