diff --git a/.typos.toml b/.typos.toml index a6135e18..2f7d7769 100644 --- a/.typos.toml +++ b/.typos.toml @@ -4,3 +4,6 @@ [files] extend-exclude = ["CHANGELOG.md"] + +[default.extend-identifiers] +PnP = "PnP" diff --git a/src/lib.rs b/src/lib.rs index b3e38cfc..2901906a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -132,7 +132,14 @@ impl ResolverGeneric { #[must_use] pub fn new(options: ResolveOptions) -> Self { let options = options.sanitize(); - let cache = Self::new_cache(&options); + 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, cache } } @@ -147,7 +154,13 @@ impl ResolverGeneric { let options = options.sanitize(); cfg_if::cfg_if! { if #[cfg(feature = "yarn_pnp")] { - let cache = Self::new_cache(&options); + let cache = if (options.yarn_pnp && !self.options.yarn_pnp) + || (!options.yarn_pnp && self.options.yarn_pnp) + { + Arc::new(Cache::new(Fs::new(options.yarn_pnp))) + } else { + Arc::clone(&self.cache) + }; } else { let cache = Arc::clone(&self.cache); } @@ -155,18 +168,6 @@ impl ResolverGeneric { Self { options, cache } } - #[allow(unused)] - fn new_cache(options: &ResolveOptions) -> Arc> { - cfg_if::cfg_if! { - if #[cfg(feature = "yarn_pnp")] { - let fs = Fs::new(options.yarn_pnp); - } else { - let fs = Fs::new(); - } - } - Arc::new(Cache::new(fs)) - } - /// Returns the options. #[must_use] pub const fn options(&self) -> &ResolveOptions { @@ -180,6 +181,13 @@ impl ResolverGeneric { self.cache.clear(); } + /// Check if two resolvers share the same cache (for testing). + #[cfg(test)] + #[allow(dead_code)] + pub(crate) fn shares_cache_with(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.cache, &other.cache) + } + /// Resolve `specifier` at an absolute path to a `directory`. /// /// A specifier is the string passed to require or import, i.e. `require("specifier")` or `import "specifier"`. diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index 7934c3bf..49d365ed 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -243,3 +243,112 @@ fn test_non_pnp_enabled_base() { )) ); } + +#[test] +fn test_cache_preserved_when_not_toggling_yarn_pnp() { + let fixture = super::fixture_root().join("pnp"); + + // Start with a PnP-enabled resolver + let base_resolver = Resolver::new(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: true, + extensions: vec![".js".into()], + condition_names: vec!["import".into()], + ..ResolveOptions::default() + }); + + // Clone with different options but keeping yarn_pnp: true + // With the bug from 9ae9056, this would create a new cache unnecessarily + let cloned_resolver = base_resolver.clone_with_options(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: true, + extensions: vec![".js".into(), ".json".into()], // Different extensions + condition_names: vec!["import".into()], + ..ResolveOptions::default() + }); + + // The cache should be shared (same Arc pointer) + // This assertion would FAIL with the buggy code from 9ae9056 + assert!( + base_resolver.shares_cache_with(&cloned_resolver), + "Cache should be preserved when yarn_pnp is not toggled" + ); + + // Verify both resolvers still work correctly + assert_eq!( + cloned_resolver.resolve(&fixture, "is-even").map(|r| r.full_path()), + Ok(fixture.join( + ".yarn/cache/is-even-npm-1.0.0-9f726520dc-2728cc2f39.zip/node_modules/is-even/index.js" + )) + ); +} + +#[test] +fn test_cache_recreated_when_toggling_yarn_pnp_on() { + let fixture = super::fixture_root().join("pnp"); + + // Start with a non-PnP resolver + let base_resolver = Resolver::new(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: false, + extensions: vec![".js".into()], + ..ResolveOptions::default() + }); + + // Clone with yarn_pnp: true (toggle on) + let cloned_resolver = base_resolver.clone_with_options(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: true, + extensions: vec![".js".into()], + condition_names: vec!["import".into()], + ..ResolveOptions::default() + }); + + // The cache should NOT be shared (different Arc) + assert!( + !base_resolver.shares_cache_with(&cloned_resolver), + "Cache should be recreated when toggling yarn_pnp on" + ); + + // And the new resolver should work with PnP + assert_eq!( + cloned_resolver.resolve(&fixture, "is-even").map(|r| r.full_path()), + Ok(fixture.join( + ".yarn/cache/is-even-npm-1.0.0-9f726520dc-2728cc2f39.zip/node_modules/is-even/index.js" + )) + ); +} + +#[test] +fn test_cache_recreated_when_toggling_yarn_pnp_off() { + let fixture = super::fixture_root().join("pnp"); + + // Start with a PnP-enabled resolver + let base_resolver = Resolver::new(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: true, + extensions: vec![".js".into()], + condition_names: vec!["import".into()], + ..ResolveOptions::default() + }); + + // Clone with yarn_pnp: false (toggle off) + let cloned_resolver = base_resolver.clone_with_options(ResolveOptions { + cwd: Some(fixture.clone()), + yarn_pnp: false, + extensions: vec![".js".into()], + ..ResolveOptions::default() + }); + + // The cache should NOT be shared (different Arc) + assert!( + !base_resolver.shares_cache_with(&cloned_resolver), + "Cache should be recreated when toggling yarn_pnp off" + ); + + // Verify the cloned resolver works without PnP + assert_eq!( + cloned_resolver.resolve(&fixture, "is-even").map(|r| r.full_path()), + Err(crate::ResolveError::NotFound("is-even".to_string())) + ); +}