clippy and finish

cli/factory.rs

@@ -643,6 +643,8 @@ impl CliFactory {
         self.workspace_factory()?.clone(),
         ResolverFactoryOptions {
           conditions_from_resolution_mode: Default::default(),
+          // todo(dsherret): configure a resolution cache
+          node_resolution_cache: None,
           no_sloppy_imports_cache: false,
           npm_system_info: self.flags.subcommand.npm_system_info(),
           specified_import_map: Some(Box::new(CliSpecifiedImportMapProvider {

cli/lib/worker.rs

@@ -7,6 +7,8 @@ use std::sync::Arc;
 
 use deno_core::error::JsError;
 use deno_node::NodeRequireLoaderRc;
+use deno_path_util::url_from_file_path;
+use deno_path_util::url_to_file_path;
 use deno_resolver::npm::DenoInNpmPackageChecker;
 use deno_resolver::npm::NpmResolver;
 use deno_runtime::colors;
@@ -136,6 +138,9 @@ pub fn create_isolate_create_params() -> Option<v8::CreateParams> {
 
 #[derive(Debug, thiserror::Error, deno_error::JsError)]
 pub enum ResolveNpmBinaryEntrypointError {
+  #[class(inherit)]
+  #[error(transparent)]
+  PathToUrl(#[from] deno_path_util::PathToUrlError),
   #[class(inherit)]
   #[error(transparent)]
   ResolvePkgJsonBinExport(ResolvePkgJsonBinExportError),
@@ -526,13 +531,13 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
       .node_resolver
       .resolve_binary_export(package_folder, sub_path)
     {
-      Ok(path) => Ok(path),
+      Ok(path) => Ok(url_from_file_path(&path)?),
       Err(original_err) => {
         // if the binary entrypoint was not found, fallback to regular node resolution
         let result =
           self.resolve_binary_entrypoint_fallback(package_folder, sub_path);
         match result {
-          Ok(Some(specifier)) => Ok(specifier),
+          Ok(Some(path)) => Ok(url_from_file_path(&path)?),
           Ok(None) => {
             Err(ResolveNpmBinaryEntrypointError::ResolvePkgJsonBinExport(
               original_err,
@@ -574,12 +579,18 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
       .map_err(
         ResolveNpmBinaryEntrypointFallbackError::PackageSubpathResolve,
       )?;
-    let Ok(path) = specifier.into_path() else {
+    let path = match specifier {
-      Err(ResolveNpmBinaryEntrypointFallbackError::ModuleNotFound(
+      UrlOrPath::Url(ref url) => match url_to_file_path(url) {
-        specifier,
+        Ok(path) => path,
-      ))
+        Err(_) => {
+          return Err(ResolveNpmBinaryEntrypointFallbackError::ModuleNotFound(
+            specifier,
+          ));
+        }
+      },
+      UrlOrPath::Path(path) => path,
     };
-    if self.shared.sys.fs_exists_no_err(p) {
+    if self.shared.sys.fs_exists_no_err(&path) {
       Ok(Some(path))
     } else {
       Err(ResolveNpmBinaryEntrypointFallbackError::ModuleNotFound(

cli/lsp/resolver.rs

@@ -36,6 +36,8 @@ use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
 use indexmap::IndexMap;
+use node_resolver::cache::NodeResolutionSys;
+use node_resolver::cache::NodeResolutionThreadLocalCache;
 use node_resolver::DenoIsBuiltInNodeModuleChecker;
 use node_resolver::NodeResolutionKind;
 use node_resolver::PackageJsonThreadLocalCache;
@@ -207,6 +209,8 @@ impl LspScopeResolver {
                   NodeResolutionKind::Execution,
                 )
               })
+              .ok()?
+              .into_url()
               .ok()?,
           ))
           .0;
@@ -257,7 +261,7 @@ impl LspScopeResolver {
                 root_node_modules_dir: byonm_npm_resolver
                   .root_node_modules_path()
                   .map(|p| p.to_path_buf()),
-                sys: CliSys::default(),
+                sys: factory.node_resolution_sys.clone(),
                 pkg_json_resolver: self.pkg_json_resolver.clone(),
               },
             )
@@ -522,6 +526,8 @@ impl LspResolver {
           resolution_mode,
           NodeResolutionKind::Types,
         )
+        .ok()?
+        .into_url()
         .ok()?,
     )))
   }
@@ -669,6 +675,7 @@ struct ResolverFactoryServices {
 struct ResolverFactory<'a> {
   config_data: Option<&'a Arc<ConfigData>>,
   pkg_json_resolver: Arc<CliPackageJsonResolver>,
+  node_resolution_sys: NodeResolutionSys<CliSys>,
   sys: CliSys,
   services: ResolverFactoryServices,
 }
@@ -684,6 +691,10 @@ impl<'a> ResolverFactory<'a> {
     Self {
       config_data,
       pkg_json_resolver,
+      node_resolution_sys: NodeResolutionSys::new(
+        sys.clone(),
+        Some(Arc::new(NodeResolutionThreadLocalCache)),
+      ),
       sys,
       services: Default::default(),
     }
@@ -702,7 +713,7 @@ impl<'a> ResolverFactory<'a> {
     let sys = CliSys::default();
     let options = if enable_byonm {
       CliNpmResolverCreateOptions::Byonm(CliByonmNpmResolverCreateOptions {
-        sys,
+        sys: self.node_resolution_sys.clone(),
         pkg_json_resolver: self.pkg_json_resolver.clone(),
         root_node_modules_dir: self.config_data.and_then(|config_data| {
           config_data.node_modules_dir.clone().or_else(|| {
@@ -926,7 +937,7 @@ impl<'a> ResolverFactory<'a> {
           DenoIsBuiltInNodeModuleChecker,
           npm_resolver.clone(),
           self.pkg_json_resolver.clone(),
-          self.sys.clone(),
+          self.node_resolution_sys.clone(),
           node_resolver::ConditionsFromResolutionMode::default(),
         )))
       })

cli/lsp/tsc.rs

@@ -49,6 +49,7 @@ use indexmap::IndexMap;
 use indexmap::IndexSet;
 use lazy_regex::lazy_regex;
 use log::error;
+use node_resolver::cache::NodeResolutionThreadLocalCache;
 use node_resolver::ResolutionMode;
 use once_cell::sync::Lazy;
 use regex::Captures;
@@ -4626,6 +4627,7 @@ fn op_resolve_inner(
     })
     .collect();
   state.performance.measure(mark);
+  NodeResolutionThreadLocalCache::clear();
   Ok(specifiers)
 }
 

cli/module_loader.rs

@@ -666,7 +666,12 @@ impl<TGraphContainer: ModuleGraphContainer>
             ResolutionMode::Import,
             NodeResolutionKind::Execution,
           )
-          .map_err(|e| JsErrorBox::from_err(e).into());
+          .map_err(|e| JsErrorBox::from_err(e).into())
+          .and_then(|url_or_path| {
+            url_or_path
+              .into_url()
+              .map_err(|e| JsErrorBox::from_err(e).into())
+          });
       }
     }
 
@@ -695,6 +700,8 @@ impl<TGraphContainer: ModuleGraphContainer>
               source,
             })
           })?
+          .into_url()
+          .map_err(JsErrorBox::from_err)?
       }
       Some(Module::Node(module)) => module.specifier.clone(),
       Some(Module::Js(module)) => module.specifier.clone(),

cli/npm/installer/local.rs

@@ -2,6 +2,7 @@
 
 //! Code for local node_modules resolution.
 
+use std::borrow::Cow;
 use std::cell::RefCell;
 use std::cmp::Ordering;
 use std::collections::hash_map::Entry;
@@ -581,7 +582,7 @@ async fn sync_resolution_with_fs(
       symlink_package_dir(
         &local_registry_package_path,
         &join_package_name(
-          Cow::Owned(deno_node_modules_dir),
+          Cow::Borrowed(&deno_node_modules_dir),
           &package.id.nv.name,
         ),
       )?;

cli/rt/run.rs

@@ -73,6 +73,7 @@ use deno_runtime::WorkerExecutionMode;
 use deno_runtime::WorkerLogLevel;
 use deno_semver::npm::NpmPackageReqReference;
 use node_resolver::analyze::NodeCodeTranslator;
+use node_resolver::cache::NodeResolutionSys;
 use node_resolver::errors::ClosestPkgJsonError;
 use node_resolver::DenoIsBuiltInNodeModuleChecker;
 use node_resolver::NodeResolutionKind;
@@ -224,7 +225,10 @@ impl ModuleLoader for EmbeddedModuleLoader {
             referrer_kind,
             NodeResolutionKind::Execution,
           )
-          .map_err(JsErrorBox::from_err)?,
+          .map_err(JsErrorBox::from_err)
+          .and_then(|url_or_path| {
+            url_or_path.into_url().map_err(JsErrorBox::from_err)
+          })?,
       ),
       Ok(MappedResolution::PackageJson {
         dep_result,
@@ -235,17 +239,22 @@ impl ModuleLoader for EmbeddedModuleLoader {
         .as_ref()
         .map_err(|e| JsErrorBox::from_err(e.clone()))?
       {
-        PackageJsonDepValue::Req(req) => self
+        PackageJsonDepValue::Req(req) => Ok(
-          .shared
+          self
-          .npm_req_resolver
+            .shared
-          .resolve_req_with_sub_path(
+            .npm_req_resolver
-            req,
+            .resolve_req_with_sub_path(
-            sub_path.as_deref(),
+              req,
-            &referrer,
+              sub_path.as_deref(),
-            referrer_kind,
+              &referrer,
-            NodeResolutionKind::Execution,
+              referrer_kind,
-          )
+              NodeResolutionKind::Execution,
-          .map_err(|e| JsErrorBox::from_err(e).into()),
+            )
+            .map_err(JsErrorBox::from_err)
+            .and_then(|url_or_path| {
+              url_or_path.into_url().map_err(JsErrorBox::from_err)
+            })?,
+        ),
         PackageJsonDepValue::Workspace(version_req) => {
           let pkg_folder = self
             .shared
@@ -266,7 +275,10 @@ impl ModuleLoader for EmbeddedModuleLoader {
                 referrer_kind,
                 NodeResolutionKind::Execution,
               )
-              .map_err(JsErrorBox::from_err)?,
+              .map_err(JsErrorBox::from_err)
+              .and_then(|url_or_path| {
+                url_or_path.into_url().map_err(JsErrorBox::from_err)
+              })?,
           )
         }
       },
@@ -285,7 +297,10 @@ impl ModuleLoader for EmbeddedModuleLoader {
                 referrer_kind,
                 NodeResolutionKind::Execution,
               )
-              .map_err(JsErrorBox::from_err)?,
+              .map_err(JsErrorBox::from_err)
+              .and_then(|url_or_path| {
+                url_or_path.into_url().map_err(JsErrorBox::from_err)
+              })?,
           );
         }
 
@@ -674,6 +689,7 @@ pub async fn run(
     };
     NpmRegistryReadPermissionChecker::new(sys.clone(), mode)
   };
+  let node_resolution_sys = NodeResolutionSys::new(sys.clone(), None);
   let (in_npm_pkg_checker, npm_resolver) = match metadata.node_modules {
     Some(NodeModules::Managed { node_modules_dir }) => {
       // create an npmrc that uses the fake npm_registry_url to resolve packages
@@ -723,7 +739,7 @@ pub async fn run(
         DenoInNpmPackageChecker::new(CreateInNpmPkgCheckerOptions::Byonm);
       let npm_resolver = NpmResolver::<DenoRtSys>::new::<DenoRtSys>(
         NpmResolverCreateOptions::Byonm(ByonmNpmResolverCreateOptions {
-          sys: sys.clone(),
+          sys: node_resolution_sys.clone(),
           pkg_json_resolver: pkg_json_resolver.clone(),
           root_node_modules_dir,
         }),
@@ -767,7 +783,7 @@ pub async fn run(
     DenoIsBuiltInNodeModuleChecker,
     npm_resolver.clone(),
     pkg_json_resolver.clone(),
-    sys.clone(),
+    node_resolution_sys,
     node_resolver::ConditionsFromResolutionMode::default(),
   ));
   let cjs_tracker = Arc::new(CjsTracker::new(

cli/tools/check.rs

@@ -667,7 +667,7 @@ fn resolve_npm_nv_ref(
       node_resolver::NodeResolutionKind::Types,
     )
     .ok()?;
-  Some(resolved)
+  resolved.into_url().ok()
 }
 
 /// Matches the `@ts-check` pragma.

cli/tools/info.rs

@@ -75,13 +75,17 @@ pub async fn info(
           target_pkg_json,
           sub_path,
           ..
-        } => Some(node_resolver.resolve_package_subpath_from_deno_module(
+        } => Some(
-          target_pkg_json.clone().dir_path(),
+          node_resolver
-          sub_path.as_deref(),
+            .resolve_package_subpath_from_deno_module(
-          Some(&cwd_url),
+              target_pkg_json.clone().dir_path(),
-          node_resolver::ResolutionMode::Import,
+              sub_path.as_deref(),
-          node_resolver::NodeResolutionKind::Execution,
+              Some(&cwd_url),
-        )?),
+              node_resolver::ResolutionMode::Import,
+              node_resolver::NodeResolutionKind::Execution,
+            )?
+            .into_url()?,
+        ),
         deno_config::workspace::MappedResolution::PackageJson {
           alias,
           sub_path,
@@ -94,13 +98,17 @@ pub async fn info(
                 alias,
                 version_req,
               )?;
-            Some(node_resolver.resolve_package_subpath_from_deno_module(
+            Some(
-              pkg_folder,
+              node_resolver
-              sub_path.as_deref(),
+                .resolve_package_subpath_from_deno_module(
-              Some(&cwd_url),
+                  pkg_folder,
-              node_resolver::ResolutionMode::Import,
+                  sub_path.as_deref(),
-              node_resolver::NodeResolutionKind::Execution,
+                  Some(&cwd_url),
-            )?)
+                  node_resolver::ResolutionMode::Import,
+                  node_resolver::NodeResolutionKind::Execution,
+                )?
+                .into_url()?,
+            )
           }
           deno_package_json::PackageJsonDepValue::Req(req) => {
             Some(ModuleSpecifier::parse(&format!(

cli/tsc/mod.rs

@@ -706,6 +706,9 @@ pub enum ResolveError {
   )]
   ModuleResolution(#[from] deno_core::ModuleResolutionError),
   #[class(inherit)]
+  #[error(transparent)]
+  FilePathToUrl(#[from] deno_path_util::PathToUrlError),
+  #[class(inherit)]
   #[error("{0}")]
   PackageSubpathResolve(PackageSubpathResolveError),
   #[class(inherit)]
@@ -937,7 +940,7 @@ fn resolve_graph_specifier_types(
             NodeResolutionKind::Types,
           );
         let maybe_url = match res_result {
-          Ok(url) => Some(url),
+          Ok(path_or_url) => Some(path_or_url.into_url()?),
           Err(err) => match err.code() {
             NodeJsErrorCode::ERR_TYPES_NOT_FOUND
             | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None,
@@ -965,6 +968,9 @@ fn resolve_graph_specifier_types(
 
 #[derive(Debug, Error, deno_error::JsError)]
 pub enum ResolveNonGraphSpecifierTypesError {
+  #[class(inherit)]
+  #[error(transparent)]
+  FilePathToUrl(#[from] deno_path_util::PathToUrlError),
   #[class(inherit)]
   #[error(transparent)]
   ResolvePkgFolderFromDenoReq(#[from] ResolvePkgFolderFromDenoReqError),
@@ -1019,7 +1025,7 @@ fn resolve_non_graph_specifier_types(
       NodeResolutionKind::Types,
     );
     let maybe_url = match res_result {
-      Ok(url) => Some(url),
+      Ok(url_or_path) => Some(url_or_path.into_url()?),
       Err(err) => match err.code() {
         NodeJsErrorCode::ERR_TYPES_NOT_FOUND
         | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None,

ext/node/ops/require.rs

@@ -23,6 +23,7 @@ use node_resolver::InNpmPackageChecker;
 use node_resolver::NodeResolutionKind;
 use node_resolver::NpmPackageFolderResolver;
 use node_resolver::ResolutionMode;
+use node_resolver::UrlOrPath;
 use node_resolver::UrlOrPathRef;
 use node_resolver::REQUIRE_CONDITIONS;
 use sys_traits::FsCanonicalize;
@@ -278,11 +279,12 @@ pub fn op_require_resolve_deno_dir<
     TSys,
   >>();
 
+  let path = PathBuf::from(parent_filename);
   Ok(
     resolver
       .resolve_package_folder_from_package(
         &request,
-        &UrlOrPathRef::new_path(PathBuf::from(parent_filename))),
+        &UrlOrPathRef::from_path(&path),
       )
       .ok()
       .map(|p| p.to_string_lossy().into_owned()),
@@ -488,9 +490,7 @@ pub fn op_require_try_self<
 
   let pkg_json_resolver = state.borrow::<PackageJsonResolverRc<TSys>>();
   let pkg = pkg_json_resolver
-    .get_closest_package_json(&PathBuf::from(
+    .get_closest_package_json(&PathBuf::from(parent_path.unwrap()))
-      parent_path.unwrap(),
-    ))
     .ok()
     .flatten();
   if pkg.is_none() {
@@ -516,13 +516,13 @@ pub fn op_require_try_self<
     return Ok(None);
   }
 
-  let referrer = deno_core::url::Url::from_file_path(&pkg.path).unwrap();
   if let Some(exports) = &pkg.exports {
     let node_resolver = state.borrow::<NodeResolverRc<
       TInNpmPackageChecker,
       TNpmPackageFolderResolver,
       TSys,
     >>();
+    let referrer = UrlOrPathRef::from_path(&pkg.path);
     let r = node_resolver.package_exports_resolve(
       &pkg.path,
       &expansion,
@@ -532,11 +532,7 @@ pub fn op_require_try_self<
       REQUIRE_CONDITIONS,
       NodeResolutionKind::Execution,
     )?;
-    Ok(Some(if r.scheme() == "file" {
+    Ok(Some(url_or_path_to_string(r)?))
-      url_to_file_path_string(&r)?
-    } else {
-      r.to_string()
-    }))
   } else {
     Ok(None)
   }
@@ -628,22 +624,21 @@ pub fn op_require_resolve_exports<
   let referrer = if parent_path.is_empty() {
     None
   } else {
-    Some(Url::from_file_path(parent_path).unwrap())
+    Some(PathBuf::from(parent_path))
   };
   let r = node_resolver.package_exports_resolve(
     &pkg.path,
     &format!(".{expansion}"),
     exports,
-    referrer.as_ref(),
+    referrer
+      .as_ref()
+      .map(|r| UrlOrPathRef::from_path(r))
+      .as_ref(),
     ResolutionMode::Require,
     REQUIRE_CONDITIONS,
     NodeResolutionKind::Execution,
   )?;
-  Ok(Some(if r.scheme() == "file" {
+  Ok(Some(url_or_path_to_string(r)?))
-    url_to_file_path_string(&r)?
-  } else {
-    r.to_string()
-  }))
 }
 
 deno_error::js_error_wrapper!(
@@ -702,8 +697,7 @@ pub fn op_require_package_imports_resolve<
   let referrer_path = ensure_read_permission::<P>(state, &referrer_path)
     .map_err(RequireErrorKind::Permission)?;
   let pkg_json_resolver = state.borrow::<PackageJsonResolverRc<TSys>>();
-  let Some(pkg) = pkg_json_resolver
+  let Some(pkg) = pkg_json_resolver.get_closest_package_json(&referrer_path)?
-    .get_closest_package_json(&referrer_path)?
   else {
     return Ok(None);
   };
@@ -714,16 +708,15 @@ pub fn op_require_package_imports_resolve<
       TNpmPackageFolderResolver,
       TSys,
     >>();
-    let referrer_url = Url::from_file_path(&referrer_filename).unwrap();
     let url = node_resolver.package_imports_resolve(
       &request,
-      Some(&referrer_url),
+      Some(&UrlOrPathRef::from_path(&referrer_path)),
       ResolutionMode::Require,
       Some(&pkg),
       REQUIRE_CONDITIONS,
       NodeResolutionKind::Execution,
     )?;
-    Ok(Some(url_to_file_path_string(&url)?))
+    Ok(Some(url_or_path_to_string(url)?))
   } else {
     Ok(None)
   }
@@ -739,11 +732,6 @@ pub fn op_require_break_on_next_statement(state: Rc<RefCell<OpState>>) {
   inspector.wait_for_session_and_break_on_next_statement()
 }
 
-fn url_to_file_path_string(url: &Url) -> Result<String, RequireError> {
-  let file_path = url_to_file_path(url)?;
-  Ok(file_path.to_string_lossy().into_owned())
-}
-
 #[op2(fast)]
 pub fn op_require_can_parse_as_esm(
   scope: &mut v8::HandleScope,
@@ -769,3 +757,13 @@ pub fn op_require_can_parse_as_esm(
   let mut source = v8::script_compiler::Source::new(source, Some(&origin));
   v8::script_compiler::compile_module(scope, &mut source).is_some()
 }
+
+fn url_or_path_to_string(
+  url: UrlOrPath,
+) -> Result<String, deno_path_util::UrlToFilePathError> {
+  if url.is_file() {
+    Ok(url.into_path()?.to_string_lossy().to_string())
+  } else {
+    Ok(url.to_string_lossy().to_string())
+  }
+}

resolvers/deno/factory.rs

@@ -23,7 +23,7 @@ use deno_npm::NpmSystemInfo;
 use deno_path_util::fs::canonicalize_path_maybe_not_exists;
 use deno_path_util::normalize_path;
 use futures::future::FutureExt;
-use node_resolver::cache::SysCache;
+use node_resolver::cache::NodeResolutionSys;
 use node_resolver::ConditionsFromResolutionMode;
 use node_resolver::DenoIsBuiltInNodeModuleChecker;
 use node_resolver::NodeResolver;
@@ -556,6 +556,7 @@ pub struct ResolverFactoryOptions {
   pub conditions_from_resolution_mode: ConditionsFromResolutionMode,
   pub no_sloppy_imports_cache: bool,
   pub npm_system_info: NpmSystemInfo,
+  pub node_resolution_cache: Option<node_resolver::NodeResolutionCacheRc>,
   pub package_json_cache: Option<node_resolver::PackageJsonCacheRc>,
   pub package_json_dep_resolution: Option<PackageJsonDepResolution>,
   pub specified_import_map: Option<Box<dyn SpecifiedImportMapProvider>>,
@@ -585,7 +586,7 @@ pub struct ResolverFactory<
     + 'static,
 > {
   options: ResolverFactoryOptions,
-  sys: SysCache<TSys>,
+  sys: NodeResolutionSys<TSys>,
   deno_resolver: async_once_cell::OnceCell<DefaultDenoResolverRc<TSys>>,
   in_npm_package_checker: Deferred<DenoInNpmPackageChecker>,
   node_resolver: Deferred<
@@ -641,8 +642,10 @@ impl<
     options: ResolverFactoryOptions,
   ) -> Self {
     Self {
-      options,
+      sys: NodeResolutionSys::new(
-      sys: SysCache::new(workspace_factory.sys.clone()),
+        workspace_factory.sys.clone(),
+        options.node_resolution_cache.clone(),
+      ),
       deno_resolver: Default::default(),
       in_npm_package_checker: Default::default(),
       node_resolver: Default::default(),
@@ -653,6 +656,7 @@ impl<
       sloppy_imports_resolver: Default::default(),
       workspace_factory,
       workspace_resolver: Default::default(),
+      options,
     }
   }
 

resolvers/deno/npm/byonm.rs

@@ -11,7 +11,7 @@ use deno_path_util::url_to_file_path;
 use deno_semver::package::PackageReq;
 use deno_semver::StackString;
 use deno_semver::Version;
-use node_resolver::cache::SysCache;
+use node_resolver::cache::NodeResolutionSys;
 use node_resolver::errors::PackageFolderResolveError;
 use node_resolver::errors::PackageFolderResolveIoError;
 use node_resolver::errors::PackageJsonLoadError;
@@ -49,7 +49,7 @@ pub enum ByonmResolvePkgFolderFromDenoReqError {
 pub struct ByonmNpmResolverCreateOptions<TSys: FsRead> {
   // todo(dsherret): investigate removing this
   pub root_node_modules_dir: Option<PathBuf>,
-  pub sys: SysCache<TSys>,
+  pub sys: NodeResolutionSys<TSys>,
   pub pkg_json_resolver: PackageJsonResolverRc<TSys>,
 }
 
@@ -61,7 +61,7 @@ pub type ByonmNpmResolverRc<TSys> =
 pub struct ByonmNpmResolver<
   TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir,
 > {
-  sys: SysCache<TSys>,
+  sys: NodeResolutionSys<TSys>,
   pkg_json_resolver: PackageJsonResolverRc<TSys>,
   root_node_modules_dir: Option<PathBuf>,
 }
@@ -137,7 +137,7 @@ impl<TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir>
     referrer: &Url,
   ) -> Result<PathBuf, ByonmResolvePkgFolderFromDenoReqError> {
     fn node_resolve_dir<TSys: FsCanonicalize + FsMetadata>(
-      sys: &SysCache<TSys>,
+      sys: &NodeResolutionSys<TSys>,
       alias: &str,
       start_dir: &Path,
     ) -> std::io::Result<Option<PathBuf>> {
@@ -147,8 +147,7 @@ impl<TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir>
         if sys.is_dir(&sub_dir) {
           return Ok(Some(
             deno_path_util::fs::canonicalize_path_maybe_not_exists(
-              sys.sys(),
+              sys, &sub_dir,
-              &sub_dir,
             )?,
           ));
         }
@@ -307,7 +306,7 @@ impl<TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir>
     // now check if node_modules/.deno/ matches this constraint
     let root_node_modules_dir = self.root_node_modules_dir.as_ref()?;
     let node_modules_deno_dir = root_node_modules_dir.join(".deno");
-    let Ok(entries) = self.sys.sys().fs_read_dir(&node_modules_deno_dir) else {
+    let Ok(entries) = self.sys.fs_read_dir(&node_modules_deno_dir) else {
       return None;
     };
     let search_prefix = format!(
@@ -344,8 +343,7 @@ impl<TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir>
       if let Some(tag) = req.version_req.tag() {
         let initialized_file =
           node_modules_deno_dir.join(&entry_name).join(".initialized");
-        let Ok(contents) =
+        let Ok(contents) = self.sys.fs_read_to_string_lossy(&initialized_file)
-          self.sys.sys().fs_read_to_string_lossy(&initialized_file)
         else {
           continue;
         };
@@ -388,7 +386,7 @@ impl<TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir>
     referrer: &UrlOrPathRef,
   ) -> Result<PathBuf, PackageFolderResolveError> {
     fn inner<TSys: FsMetadata>(
-      sys: &SysCache<TSys>,
+      sys: &NodeResolutionSys<TSys>,
       name: &str,
       referrer: &UrlOrPathRef,
     ) -> Result<PathBuf, PackageFolderResolveError> {
@@ -422,7 +420,7 @@ impl<TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir>
     }
 
     let path = inner(&self.sys, name, referrer)?;
-    self.sys.canonicalize(&path).map_err(|err| {
+    self.sys.fs_canonicalize(&path).map_err(|err| {
       PackageFolderResolveIoError {
         package_name: name.to_string(),
         referrer: referrer.display(),

resolvers/deno/npm/managed/local.rs

@@ -15,7 +15,6 @@ use node_resolver::errors::PackageFolderResolveIoError;
 use node_resolver::errors::PackageNotFoundError;
 use node_resolver::errors::ReferrerNotFoundError;
 use node_resolver::NpmPackageFolderResolver;
-use node_resolver::UrlOrPath;
 use node_resolver::UrlOrPathRef;
 use sys_traits::FsCanonicalize;
 use sys_traits::FsMetadata;

resolvers/deno/npm/managed/mod.rs

@@ -19,7 +19,6 @@ use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
 use node_resolver::InNpmPackageChecker;
 use node_resolver::NpmPackageFolderResolver;
-use node_resolver::UrlOrPath;
 use node_resolver::UrlOrPathRef;
 use sys_traits::FsCanonicalize;
 use sys_traits::FsMetadata;

resolvers/deno/npm/mod.rs

@@ -426,14 +426,12 @@ impl<
               ),
               PackageResolveErrorKind::PackageFolderResolve(err) => {
                 match err.as_kind() {
-                  PackageFolderResolveErrorKind::PathToUrl(err) => {
+                  PackageFolderResolveErrorKind::PathToUrl(err) => Err(
-                    return Err(
+                    ResolveIfForNpmPackageErrorKind::NodeResolve(
-                      ResolveIfForNpmPackageErrorKind::NodeResolve(
+                      NodeResolveErrorKind::PathToUrl(err.clone()).into_box(),
-                        NodeResolveErrorKind::PathToUrl(err.clone()).into_box(),
-                      )
-                      .into_box(),
                     )
-                  }
+                    .into_box(),
+                  ),
                   PackageFolderResolveErrorKind::Io(
                     PackageFolderResolveIoError { package_name, .. },
                   )

resolvers/node/analyze.rs

@@ -7,7 +7,6 @@ use std::path::Path;
 use std::path::PathBuf;
 
 use deno_error::JsErrorBox;
-use deno_path_util::url_from_file_path;
 use deno_path_util::url_to_file_path;
 use futures::future::LocalBoxFuture;
 use futures::stream::FuturesUnordered;
@@ -369,12 +368,12 @@ impl<
       todo!();
     }
 
-    let referrer = UrlOrPathRef::new_url(referrer);
+    let referrer = UrlOrPathRef::from_url(referrer);
     let referrer_path = referrer.path().unwrap();
     if specifier.starts_with("./") || specifier.starts_with("../") {
       if let Some(parent) = referrer_path.parent() {
         return self
-          .file_extension_probe(parent.join(specifier), &referrer_path)
+          .file_extension_probe(parent.join(specifier), referrer_path)
           .map(|p| Some(UrlOrPath::Path(p)));
       } else {
         todo!();
@@ -446,7 +445,7 @@ impl<
           return Ok(Some(UrlOrPath::Path(d.join("index.js").clean())));
         }
         return self
-          .file_extension_probe(d, &referrer_path)
+          .file_extension_probe(d, referrer_path)
           .map(|p| Some(UrlOrPath::Path(p)));
       } else if let Some(main) =
         package_json.main(deno_package_json::NodeModuleKind::Cjs)
@@ -468,13 +467,13 @@ impl<
       } else {
         parent.join("node_modules").join(specifier)
       };
-      if let Ok(path) = self.file_extension_probe(path, &referrer_path) {
+      if let Ok(path) = self.file_extension_probe(path, referrer_path) {
         return Ok(Some(UrlOrPath::Path(path)));
       }
       last = parent;
     }
 
-    Err(not_found(specifier, &referrer_path))
+    Err(not_found(specifier, referrer_path))
   }
 
   fn file_extension_probe(

resolvers/node/cache.rs

@@ -1,73 +1,136 @@
+// Copyright 2018-2025 the Deno authors. MIT license.
+
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
-use std::rc::Rc;
 
+use sys_traits::BaseFsCanonicalize;
+use sys_traits::BaseFsRead;
+use sys_traits::BaseFsReadDir;
 use sys_traits::FileType;
 use sys_traits::FsCanonicalize;
 use sys_traits::FsMetadata;
 use sys_traits::FsMetadataValue;
+use sys_traits::FsRead;
+use sys_traits::FsReadDir;
 
-// todo(THIS PR): better name lol
+pub trait NodeResolutionCache:
-#[derive(Debug, Default)]
+  std::fmt::Debug + crate::sync::MaybeSend + crate::sync::MaybeSync
-pub struct SysCache<TSys> {
+{
-  sys: TSys,
+  fn get_canonicalized(
-  // todo: combine Rcs
+    &self,
-  cache: Rc<RefCell<HashMap<PathBuf, Option<FileType>>>>,
+    path: &Path,
-  canonicalize_cache: Rc<RefCell<HashMap<PathBuf, PathBuf>>>,
+  ) -> Option<Result<PathBuf, std::io::Error>>;
+  fn set_canonicalized(&self, from: PathBuf, to: &std::io::Result<PathBuf>);
+  fn get_file_type(&self, path: &Path) -> Option<Option<FileType>>;
+  fn set_file_type(&self, path: PathBuf, value: Option<FileType>);
 }
 
-impl<TSys: Clone> Clone for SysCache<TSys> {
+thread_local! {
+  static CANONICALIZED_CACHE: RefCell<HashMap<PathBuf, Option<PathBuf>>> = RefCell::new(HashMap::new());
+  static FILE_TYPE_CACHE: RefCell<HashMap<PathBuf, Option<FileType>>> = RefCell::new(HashMap::new());
+}
+
+#[derive(Debug)]
+pub struct NodeResolutionThreadLocalCache;
+
+impl NodeResolutionThreadLocalCache {
+  pub fn clear() {
+    CANONICALIZED_CACHE.with_borrow_mut(|cache| cache.clear());
+    FILE_TYPE_CACHE.with_borrow_mut(|cache| cache.clear());
+  }
+}
+
+impl NodeResolutionCache for NodeResolutionThreadLocalCache {
+  fn get_canonicalized(
+    &self,
+    path: &Path,
+  ) -> Option<Result<PathBuf, std::io::Error>> {
+    CANONICALIZED_CACHE.with_borrow(|cache| {
+      let item = cache.get(path)?;
+      Some(match item {
+        Some(value) => Ok(value.clone()),
+        None => Err(std::io::Error::new(
+          std::io::ErrorKind::NotFound,
+          "Not found.",
+        )),
+      })
+    })
+  }
+
+  fn set_canonicalized(&self, from: PathBuf, to: &std::io::Result<PathBuf>) {
+    CANONICALIZED_CACHE.with_borrow_mut(|cache| match to {
+      Ok(to) => {
+        cache.insert(from, Some(to.clone()));
+      }
+      Err(err) => {
+        if err.kind() == std::io::ErrorKind::NotFound {
+          cache.insert(from, None);
+        }
+      }
+    });
+  }
+
+  fn get_file_type(&self, path: &Path) -> Option<Option<FileType>> {
+    FILE_TYPE_CACHE.with_borrow(|cache| cache.get(path).cloned())
+  }
+
+  fn set_file_type(&self, path: PathBuf, value: Option<FileType>) {
+    FILE_TYPE_CACHE.with_borrow_mut(|cache| {
+      cache.insert(path, value);
+    })
+  }
+}
+
+#[allow(clippy::disallowed_types)]
+pub type NodeResolutionCacheRc = crate::sync::MaybeArc<dyn NodeResolutionCache>;
+
+#[derive(Debug, Default)]
+pub struct NodeResolutionSys<TSys> {
+  sys: TSys,
+  cache: Option<NodeResolutionCacheRc>,
+}
+
+impl<TSys: Clone> Clone for NodeResolutionSys<TSys> {
   fn clone(&self) -> Self {
     Self {
       sys: self.sys.clone(),
       cache: self.cache.clone(),
-      canonicalize_cache: self.canonicalize_cache.clone(),
     }
   }
 }
 
-impl<TSys: FsMetadata> SysCache<TSys> {
+impl<TSys: FsMetadata> NodeResolutionSys<TSys> {
-  pub fn new(sys: TSys) -> Self {
+  pub fn new(sys: TSys, store: Option<NodeResolutionCacheRc>) -> Self {
-    Self {
+    Self { sys, cache: store }
-      sys,
-      cache: Default::default(),
-      canonicalize_cache: Default::default(),
-    }
-  }
-
-  pub fn sys(&self) -> &TSys {
-    &self.sys
   }
 
   pub fn is_file(&self, path: &Path) -> bool {
-    match self.get(path) {
+    match self.get_file_type(path) {
       Ok(file_type) => file_type.is_file(),
       Err(_) => false,
     }
   }
 
   pub fn is_dir(&self, path: &Path) -> bool {
-    match self.get(path) {
+    match self.get_file_type(path) {
       Ok(file_type) => file_type.is_dir(),
       Err(_) => false,
     }
   }
 
   pub fn exists_(&self, path: &Path) -> bool {
-    match self.get(path) {
+    self.get_file_type(path).is_ok()
-      Ok(_) => true,
-      Err(_) => false,
-    }
   }
 
-  // todo(THIS PR): better name
+  pub fn get_file_type(&self, path: &Path) -> std::io::Result<FileType> {
-  pub fn get(&self, path: &Path) -> std::io::Result<FileType> {
     {
-      if let Some(file_type) = self.cache.borrow().get(path) {
+      if let Some(maybe_value) =
-        return match *file_type {
+        self.cache.as_ref().and_then(|c| c.get_file_type(path))
-          Some(file_type) => Ok(file_type),
+      {
+        return match maybe_value {
+          Some(value) => Ok(value),
           None => Err(std::io::Error::new(
             std::io::ErrorKind::NotFound,
             "Not found.",
@@ -77,32 +140,56 @@ impl<TSys: FsMetadata> SysCache<TSys> {
     }
     match self.sys.fs_metadata(path) {
       Ok(metadata) => {
-        self
+        if let Some(cache) = &self.cache {
-          .cache
+          cache.set_file_type(path.to_path_buf(), Some(metadata.file_type()));
-          .borrow_mut()
+        }
-          .insert(path.to_path_buf(), Some(metadata.file_type()));
         Ok(metadata.file_type())
       }
       Err(err) => {
-        self.cache.borrow_mut().insert(path.to_path_buf(), None);
+        if let Some(cache) = &self.cache {
+          cache.set_file_type(path.to_path_buf(), None);
+        }
         Err(err)
       }
     }
   }
 }
 
-impl<TSys: FsCanonicalize> SysCache<TSys> {
+impl<TSys: FsCanonicalize> BaseFsCanonicalize for NodeResolutionSys<TSys> {
-  pub fn canonicalize(&self, path: &Path) -> std::io::Result<PathBuf> {
+  fn base_fs_canonicalize(&self, from: &Path) -> std::io::Result<PathBuf> {
-    {
+    if let Some(cache) = &self.cache {
-      if let Some(path) = self.canonicalize_cache.borrow().get(path) {
+      if let Some(result) = cache.get_canonicalized(from) {
-        return Ok(path.clone());
+        return result;
       }
     }
-    let canon = self.sys.fs_canonicalize(path)?;
+    let result = self.sys.base_fs_canonicalize(from);
-    self
+    if let Some(cache) = &self.cache {
-      .canonicalize_cache
+      cache.set_canonicalized(from.to_path_buf(), &result);
-      .borrow_mut()
+    }
-      .insert(path.to_path_buf(), canon.clone());
+    result
-    Ok(canon)
+  }
+}
+
+impl<TSys: FsReadDir> BaseFsReadDir for NodeResolutionSys<TSys> {
+  type ReadDirEntry = TSys::ReadDirEntry;
+
+  #[inline(always)]
+  fn base_fs_read_dir(
+    &self,
+    path: &Path,
+  ) -> std::io::Result<
+    Box<dyn Iterator<Item = std::io::Result<Self::ReadDirEntry>> + '_>,
+  > {
+    self.sys.base_fs_read_dir(path)
+  }
+}
+
+impl<TSys: FsRead> BaseFsRead for NodeResolutionSys<TSys> {
+  #[inline(always)]
+  fn base_fs_read(
+    &self,
+    path: &Path,
+  ) -> std::io::Result<std::borrow::Cow<'static, [u8]>> {
+    self.sys.base_fs_read(path)
   }
 }

resolvers/node/errors.rs

@@ -166,7 +166,7 @@ impl NodeJsErrorCoded for PackageFolderResolveError {
       PackageFolderResolveErrorKind::PackageNotFound(e) => e.code(),
       PackageFolderResolveErrorKind::ReferrerNotFound(e) => e.code(),
       PackageFolderResolveErrorKind::Io(e) => e.code(),
-      PackageFolderResolveErrorKind::PathToUrl(e) => {
+      PackageFolderResolveErrorKind::PathToUrl(_) => {
         NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH
       }
     }

resolvers/node/lib.rs

@@ -17,6 +17,8 @@ mod sync;
 pub use builtin_modules::DenoIsBuiltInNodeModuleChecker;
 pub use builtin_modules::IsBuiltInNodeModuleChecker;
 pub use builtin_modules::DENO_SUPPORTED_BUILTIN_NODE_MODULES;
+pub use cache::NodeResolutionCache;
+pub use cache::NodeResolutionCacheRc;
 pub use deno_package_json::PackageJson;
 pub use npm::InNpmPackageChecker;
 pub use npm::NpmPackageFolderResolver;

resolvers/node/package_json.rs

@@ -9,7 +9,6 @@ use std::path::PathBuf;
 use deno_package_json::PackageJson;
 use deno_package_json::PackageJsonRc;
 use sys_traits::FsRead;
-use url::Url;
 
 use crate::errors::ClosestPkgJsonError;
 use crate::errors::PackageJsonLoadError;
@@ -51,17 +50,17 @@ pub struct PackageJsonThreadLocalCache;
 
 impl PackageJsonThreadLocalCache {
   pub fn clear() {
-    CACHE.with(|cache| cache.borrow_mut().clear());
+    CACHE.with_borrow_mut(|cache| cache.clear());
   }
 }
 
 impl deno_package_json::PackageJsonCache for PackageJsonThreadLocalCache {
   fn get(&self, path: &Path) -> Option<PackageJsonRc> {
-    CACHE.with(|cache| cache.borrow().get(path).cloned())
+    CACHE.with_borrow(|cache| cache.get(path).cloned())
   }
 
   fn set(&self, path: PathBuf, package_json: PackageJsonRc) {
-    CACHE.with(|cache| cache.borrow_mut().insert(path, package_json));
+    CACHE.with_borrow_mut(|cache| cache.insert(path, package_json));
   }
 }
 

resolvers/node/path.rs

@@ -43,6 +43,13 @@ impl UrlOrPath {
       UrlOrPath::Path(path) => deno_path_util::url_from_file_path(&path),
     }
   }
+
+  pub fn to_string_lossy(&self) -> Cow<str> {
+    match self {
+      UrlOrPath::Url(url) => Cow::Borrowed(url.as_str()),
+      UrlOrPath::Path(path) => path.to_string_lossy(),
+    }
+  }
 }
 
 impl std::fmt::Display for UrlOrPath {
@@ -51,7 +58,7 @@ impl std::fmt::Display for UrlOrPath {
       UrlOrPath::Url(url) => url.fmt(f),
       UrlOrPath::Path(path) => {
         // prefer displaying a url
-        match deno_path_util::url_from_file_path(&path) {
+        match deno_path_util::url_from_file_path(path) {
           Ok(url) => url.fmt(f),
           Err(_) => {
             write!(f, "{}", path.display())
@@ -68,14 +75,14 @@ pub struct UrlOrPathRef<'a> {
 }
 
 impl<'a> UrlOrPathRef<'a> {
-  pub fn new_path(path: &'a Path) -> Self {
+  pub fn from_path(path: &'a Path) -> Self {
     Self {
       url: Default::default(),
       path: once_cell::unsync::OnceCell::with_value(Cow::Borrowed(path)),
     }
   }
 
-  pub fn new_url(url: &'a Url) -> Self {
+  pub fn from_url(url: &'a Url) -> Self {
     Self {
       path: Default::default(),
       url: once_cell::unsync::OnceCell::with_value(Cow::Borrowed(url)),

resolvers/node/resolution.rs

@@ -8,7 +8,6 @@ use std::path::PathBuf;
 use anyhow::bail;
 use anyhow::Error as AnyError;
 use deno_package_json::PackageJson;
-use deno_path_util::url_from_file_path;
 use serde_json::Map;
 use serde_json::Value;
 use sys_traits::FileType;
@@ -17,7 +16,7 @@ use sys_traits::FsMetadata;
 use sys_traits::FsRead;
 use url::Url;
 
-use crate::cache::SysCache;
+use crate::cache::NodeResolutionSys;
 use crate::errors;
 use crate::errors::DataUrlReferrerError;
 use crate::errors::FinalizeResolutionError;
@@ -162,7 +161,7 @@ pub struct NodeResolver<
   is_built_in_node_module_checker: TIsBuiltInNodeModuleChecker,
   npm_pkg_folder_resolver: TNpmPackageFolderResolver,
   pkg_json_resolver: PackageJsonResolverRc<TSys>,
-  sys: SysCache<TSys>,
+  sys: NodeResolutionSys<TSys>,
   conditions_from_resolution_mode: ConditionsFromResolutionMode,
 }
 
@@ -184,7 +183,7 @@ impl<
     is_built_in_node_module_checker: TIsBuiltInNodeModuleChecker,
     npm_pkg_folder_resolver: TNpmPackageFolderResolver,
     pkg_json_resolver: PackageJsonResolverRc<TSys>,
-    sys: SysCache<TSys>,
+    sys: NodeResolutionSys<TSys>,
     conditions_from_resolution_mode: ConditionsFromResolutionMode,
   ) -> Self {
     Self {
@@ -254,7 +253,7 @@ impl<
     let conditions = self
       .conditions_from_resolution_mode
       .resolve(resolution_mode);
-    let referrer = UrlOrPathRef::new_url(referrer);
+    let referrer = UrlOrPathRef::from_url(referrer);
     let url = self.module_resolve(
       specifier,
       &referrer,
@@ -375,7 +374,7 @@ impl<
       path
     };
 
-    let maybe_file_type = self.sys.get(&path);
+    let maybe_file_type = self.sys.get_file_type(&path);
     match maybe_file_type {
       Ok(FileType::Dir) => Err(
         UnsupportedDirImportError {
@@ -409,7 +408,7 @@ impl<
     let package_subpath = package_subpath
       .map(|s| format!("./{s}"))
       .unwrap_or_else(|| ".".to_string());
-    let maybe_referrer = maybe_referrer.map(|r| UrlOrPathRef::new_url(r));
+    let maybe_referrer = maybe_referrer.map(UrlOrPathRef::from_url);
     let resolved_url = self.resolve_package_dir_subpath(
       package_dir,
       &package_subpath,
@@ -497,7 +496,7 @@ impl<
     conditions: &[&str],
   ) -> Result<UrlOrPath, TypesNotFoundError> {
     fn probe_extensions<TSys: FsMetadata>(
-      sys: &SysCache<TSys>,
+      sys: &NodeResolutionSys<TSys>,
       path: &Path,
       lowercase_path: &str,
       resolution_mode: ResolutionMode,
@@ -579,7 +578,7 @@ impl<
       return Ok(UrlOrPath::Path(path));
     }
     Err(TypesNotFoundError(Box::new(TypesNotFoundErrorData {
-      code_specifier: UrlOrPathRef::new_path(&path).display(),
+      code_specifier: UrlOrPathRef::from_path(&path).display(),
       maybe_referrer: maybe_referrer.map(|r| r.display()),
     })))
   }
@@ -732,7 +731,7 @@ impl<
             };
             let result = match self.package_resolve(
               &export_target,
-              &UrlOrPathRef::new_path(package_json_path),
+              &UrlOrPathRef::from_path(package_json_path),
               resolution_mode,
               conditions,
               resolution_kind,
@@ -1545,7 +1544,7 @@ impl<
     if resolution_kind.is_types() {
       Err(
         TypesNotFoundError(Box::new(TypesNotFoundErrorData {
-          code_specifier: UrlOrPathRef::new_path(&directory.join("index.js"))
+          code_specifier: UrlOrPathRef::from_path(&directory.join("index.js"))
             .display(),
           maybe_referrer: maybe_referrer.map(|r| r.display()),
         }))
@@ -1573,8 +1572,7 @@ impl<
     {
       // Specifiers in the node_modules directory are canonicalized
       // so canoncalize then check if it's in the node_modules directory.
-      let specifier =
+      let specifier = resolve_specifier_into_node_modules(&self.sys, specifier);
-        resolve_specifier_into_node_modules(self.sys.sys(), specifier);
       return Some(specifier);
     }
 
@@ -2056,7 +2054,7 @@ mod tests {
   #[test]
   fn test_parse_package_name() {
     let dummy_referrer = Url::parse("http://example.com").unwrap();
-    let dummy_referrer = UrlOrPathRef::new_url(&dummy_referrer);
+    let dummy_referrer = UrlOrPathRef::from_url(&dummy_referrer);
 
     assert_eq!(
       parse_npm_pkg_name("fetch-blob", &dummy_referrer).unwrap(),