Skip to content

Commit

Permalink
refactor: make PackageJsonCache injectable (#27800)
Browse files Browse the repository at this point in the history
This used to be complicated to do, but is now trivial.
  • Loading branch information
dsherret authored and bartlomieju committed Jan 30, 2025
1 parent 134f5e1 commit d7b1f99
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 12 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ data-encoding = "2.3.3"
data-url = "=0.3.1"
deno_cache_dir = "=0.17.0"
deno_error = "=0.5.5"
deno_package_json = { version = "0.4.0", default-features = false }
deno_package_json = { version = "=0.4.1", default-features = false }
deno_unsync = "0.4.2"
dlopen2 = "0.6.1"
ecb = "=0.1.2"
Expand Down
3 changes: 3 additions & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,9 @@ impl CliFactory {
.clone(),
})),
unstable_sloppy_imports: self.flags.unstable_config.sloppy_imports,
package_json_cache: Some(Arc::new(
node_resolver::PackageJsonThreadLocalCache,
)),
package_json_dep_resolution: match &self.flags.subcommand {
DenoSubcommand::Publish(_) => {
// the node_modules directory is not published to jsr, so resolve
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2049,7 +2049,7 @@ impl deno_config::deno_json::DenoJsonCache for DenoJsonMemCache {
}
}

#[derive(Default)]
#[derive(Debug, Default)]
struct PackageJsonMemCache(Mutex<HashMap<PathBuf, Arc<PackageJson>>>);

impl deno_package_json::PackageJsonCache for PackageJsonMemCache {
Expand Down
7 changes: 6 additions & 1 deletion cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use deno_semver::package::PackageReq;
use indexmap::IndexMap;
use node_resolver::DenoIsBuiltInNodeModuleChecker;
use node_resolver::NodeResolutionKind;
use node_resolver::PackageJsonThreadLocalCache;
use node_resolver::ResolutionMode;

use super::cache::LspCache;
Expand Down Expand Up @@ -675,7 +676,11 @@ struct ResolverFactory<'a> {
impl<'a> ResolverFactory<'a> {
pub fn new(config_data: Option<&'a Arc<ConfigData>>) -> Self {
let sys = CliSys::default();
let pkg_json_resolver = Arc::new(CliPackageJsonResolver::new(sys.clone()));
let pkg_json_resolver = Arc::new(CliPackageJsonResolver::new(
sys.clone(),
// this should be ok because we handle clearing this cache often in the LSP
Some(Arc::new(PackageJsonThreadLocalCache)),
));
Self {
config_data,
pkg_json_resolver,
Expand Down
6 changes: 5 additions & 1 deletion cli/rt/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ use node_resolver::DenoIsBuiltInNodeModuleChecker;
use node_resolver::NodeResolutionKind;
use node_resolver::NodeResolver;
use node_resolver::PackageJsonResolver;
use node_resolver::PackageJsonThreadLocalCache;
use node_resolver::ResolutionMode;

use crate::binary::DenoCompileModuleSource;
Expand Down Expand Up @@ -652,7 +653,10 @@ pub async fn run(
let root_dir_url = Arc::new(Url::from_directory_path(&root_path).unwrap());
let main_module = root_dir_url.join(&metadata.entrypoint_key).unwrap();
let npm_global_cache_dir = root_path.join(".deno_compile_node_modules");
let pkg_json_resolver = Arc::new(PackageJsonResolver::new(sys.clone()));
let pkg_json_resolver = Arc::new(PackageJsonResolver::new(
sys.clone(),
Some(Arc::new(PackageJsonThreadLocalCache)),
));
let npm_registry_permission_checker = {
let mode = match &metadata.node_modules {
Some(NodeModules::Managed {
Expand Down
6 changes: 5 additions & 1 deletion resolvers/deno/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ pub struct ResolverFactoryOptions {
pub conditions_from_resolution_mode: ConditionsFromResolutionMode,
pub no_sloppy_imports_cache: bool,
pub npm_system_info: NpmSystemInfo,
pub package_json_cache: Option<node_resolver::PackageJsonCacheRc>,
pub package_json_dep_resolution: Option<PackageJsonDepResolution>,
pub specified_import_map: Option<Box<dyn SpecifiedImportMapProvider>>,
pub unstable_sloppy_imports: bool,
Expand Down Expand Up @@ -792,7 +793,10 @@ impl<

pub fn pkg_json_resolver(&self) -> &PackageJsonResolverRc<TSys> {
self.pkg_json_resolver.get_or_init(|| {
new_rc(PackageJsonResolver::new(self.workspace_factory.sys.clone()))
new_rc(PackageJsonResolver::new(
self.workspace_factory.sys.clone(),
self.options.package_json_cache.clone(),
))
})
}

Expand Down
1 change: 1 addition & 0 deletions resolvers/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub use builtin_modules::DENO_SUPPORTED_BUILTIN_NODE_MODULES;
pub use deno_package_json::PackageJson;
pub use npm::InNpmPackageChecker;
pub use npm::NpmPackageFolderResolver;
pub use package_json::PackageJsonCacheRc;
pub use package_json::PackageJsonResolver;
pub use package_json::PackageJsonResolverRc;
pub use package_json::PackageJsonThreadLocalCache;
Expand Down
20 changes: 15 additions & 5 deletions resolvers/node/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,18 @@ use url::Url;
use crate::errors::ClosestPkgJsonError;
use crate::errors::PackageJsonLoadError;

// it would be nice if this was passed down as a ctor arg to the package.json resolver,
// but it's a little bit complicated to do that, so we just maintain a thread local cache
#[allow(clippy::disallowed_types)]
pub type PackageJsonCacheRc = crate::sync::MaybeArc<
dyn deno_package_json::PackageJsonCache
+ crate::sync::MaybeSend
+ crate::sync::MaybeSync,
>;

thread_local! {
static CACHE: RefCell<HashMap<PathBuf, PackageJsonRc>> = RefCell::new(HashMap::new());
}

#[derive(Debug)]
pub struct PackageJsonThreadLocalCache;

impl PackageJsonThreadLocalCache {
Expand All @@ -45,11 +51,12 @@ pub type PackageJsonResolverRc<TSys> =
#[derive(Debug)]
pub struct PackageJsonResolver<TSys: FsRead> {
sys: TSys,
loader_cache: Option<PackageJsonCacheRc>,
}

impl<TSys: FsRead> PackageJsonResolver<TSys> {
pub fn new(sys: TSys) -> Self {
Self { sys }
pub fn new(sys: TSys, loader_cache: Option<PackageJsonCacheRc>) -> Self {
Self { sys, loader_cache }
}

pub fn get_closest_package_json(
Expand Down Expand Up @@ -83,7 +90,10 @@ impl<TSys: FsRead> PackageJsonResolver<TSys> {
) -> Result<Option<PackageJsonRc>, PackageJsonLoadError> {
let result = PackageJson::load_from_path(
&self.sys,
Some(&PackageJsonThreadLocalCache),
self
.loader_cache
.as_deref()
.map(|cache| cache as &dyn deno_package_json::PackageJsonCache),
path,
);
match result {
Expand Down
6 changes: 6 additions & 0 deletions resolvers/node/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@ pub use inner::*;
mod inner {
#![allow(clippy::disallowed_types)]

pub use core::marker::Send as MaybeSend;
pub use core::marker::Sync as MaybeSync;
pub use std::sync::Arc as MaybeArc;
}

#[cfg(not(feature = "sync"))]
mod inner {
pub trait MaybeSync {}
impl<T> MaybeSync for T where T: ?Sized {}
pub trait MaybeSend {}
impl<T> MaybeSend for T where T: ?Sized {}
pub use std::rc::Rc as MaybeArc;
}

0 comments on commit d7b1f99

Please sign in to comment.