Repository: oxc-project/oxc-resolver
Pull request #142 (2024-04-23 16:28:49)

 lazily read package.json.browser_fields for shared resolvers

codspeed-hq[bot]: ## [CodSpeed Performance Report](https://codspeed.io/oxc-project/oxc-resolver/branches/browser-field-refactor2)

### Merging #142 will **degrade performances by 29.15%**

<sub>Comparing <code>browser-field-refactor2</code> (a5f54c0) with <code>main</code> (b887e55)</sub>



### Summary

`❌ 2` regressions



> :warning: _Please fix the performance issues or [acknowledge them on CodSpeed](https://codspeed.io/oxc-project/oxc-resolver/branches/browser-field-refactor2)._

### Benchmarks breakdown

|     | Benchmark | `main` | `browser-field-refactor2` | Change |
| --- | --------- | ----------------------- | ------------------- | ------ |
| ❌ | `resolver[multi-thread]` | 2 ms | 2.7 ms | -25.46% |
| ❌ | `resolver[single-thread]` | 1.8 ms | 2.5 ms | -29.15% |

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/142?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 96.08%. Comparing base [(`b887e55`)](https://app.codecov.io/gh/oxc-project/oxc-resolver/commit/b887e558614ef69b56246af25cd791338347dce3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project) to head [(`a5f54c0`)](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/142?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #142      +/-   ##
==========================================
+ Coverage   96.03%   96.08%   +0.05%     
==========================================
  Files          11       11              
  Lines        2272     2276       +4     
==========================================
+ Hits         2182     2187       +5     
+ Misses         90       89       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/142?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project).


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 8e25745..b48d751 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -383,20 +383,6 @@ dependencies = [
  "libloading",
 ]
 
-[[package]]
-name = "nodejs_package_json"
-version = "0.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ad444c78696e8b4de2de8a45468d0cc76630e2ed235c84e102c197b89d295a0b"
-dependencies = [
- "indexmap",
- "rustc-hash",
- "semver",
- "serde",
- "serde_json",
- "thiserror",
-]
-
 [[package]]
 name = "normalize-path"
 version = "0.2.1"
@@ -443,7 +429,6 @@ dependencies = [
  "document-features",
  "dunce",
  "json-strip-comments",
- "nodejs_package_json",
  "normalize-path",
  "once_cell",
  "rayon",
@@ -583,9 +568,6 @@ name = "semver"
 version = "1.0.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca"
-dependencies = [
- "serde",
-]
 
 [[package]]
 name = "serde"
diff --git a/Cargo.toml b/Cargo.toml
index 7247072..2f8fb71 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -85,7 +85,6 @@ once_cell = "1.19.0" # Use `std::sync::OnceLock::get_or_try_init` when it is sta
 thiserror = { version = "1.0.59" }
 json-strip-comments = { version = "1.0.2" }
 typescript_tsconfig_json = { version = "0.1.4" }
-nodejs_package_json = { version = "0.2.0" }
 
 document-features = { version = "0.2", optional = true }
 
diff --git a/src/cache.rs b/src/cache.rs
index 4507384..564e7d6 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -293,15 +293,10 @@ impl CachedPathImpl {
                 } else {
                     package_json_path.clone()
                 };
-                PackageJson::parse(
-                    package_json_path.clone(),
-                    real_path,
-                    &package_json_string,
-                    options,
-                )
-                .map(Arc::new)
-                .map(Some)
-                .map_err(|error| ResolveError::from_serde_json_error(package_json_path, &error))
+                PackageJson::parse(package_json_path.clone(), real_path, &package_json_string)
+                    .map(Arc::new)
+                    .map(Some)
+                    .map_err(|error| ResolveError::from_serde_json_error(package_json_path, &error))
             })
             .cloned();
         // https://github.com/webpack/enhanced-resolve/blob/58464fc7cb56673c9aa849e68e6300239601e615/lib/DescriptionFileUtils.js#L68-L82
diff --git a/src/lib.rs b/src/lib.rs
index 5e966f1..57cc967 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -88,7 +88,7 @@ use crate::{
     cache::{Cache, CachedPath},
     context::ResolveContext as Ctx,
     file_system::FileSystemOs,
-    package_json::ImportExportMap,
+    package_json::JSONMap,
     path::{PathUtil, SLASH_START},
     specifier::Specifier,
     tsconfig::{ProjectReference, TsConfig},
@@ -830,7 +830,11 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         ctx: &mut Ctx,
     ) -> ResolveResult {
         let path = cached_path.path();
-        let Some(new_specifier) = package_json.resolve_browser_field(path, module_specifier)?
+        let Some(new_specifier) = package_json.resolve_browser_field(
+            path,
+            module_specifier,
+            &self.options.alias_fields,
+        )?
         else {
             return Ok(None);
         };
@@ -1325,7 +1329,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     fn package_imports_exports_resolve(
         &self,
         match_key: &str,
-        match_obj: &ImportExportMap,
+        match_obj: &JSONMap,
         package_url: &Path,
         is_imports: bool,
         conditions: &[String],
diff --git a/src/package_json.rs b/src/package_json.rs
index 6a748c6..ab45f26 100644
--- a/src/package_json.rs
+++ b/src/package_json.rs
@@ -3,13 +3,11 @@
 //! Code related to export field are copied from [Parcel's resolver](https://github.com/parcel-bundler/parcel/blob/v2/packages/utils/node-resolver-rs/src/package_json.rs)
 use std::path::{Path, PathBuf};
 
-use nodejs_package_json::BrowserField;
-use serde::Deserialize;
 use serde_json::Value as JSONValue;
 
-use crate::{path::PathUtil, ResolveError, ResolveOptions};
+use crate::{path::PathUtil, ResolveError};
 
-pub type ImportExportMap = serde_json::Map<String, JSONValue>;
+pub type JSONMap = serde_json::Map<String, JSONValue>;
 
 /// Deserialized package.json
 #[derive(Debug, Default)]
@@ -20,19 +18,13 @@ pub struct PackageJson {
     /// Realpath to `package.json`. Contains the `package.json` filename.
     pub realpath: PathBuf,
 
-    raw_json: std::sync::Arc<JSONValue>,
-
     /// The "name" field defines your package's name.
     /// The "name" field can be used in addition to the "exports" field to self-reference a package using its name.
     ///
     /// <https://nodejs.org/api/packages.html#name>
     pub name: Option<String>,
 
-    /// The "browser" field is provided by a module author as a hint to javascript bundlers or component tools when packaging modules for client side use.
-    /// Multiple values are configured by [ResolveOptions::alias_fields].
-    ///
-    /// <https://github.com/defunctzombie/package-browser-field-spec>
-    pub browser_fields: Vec<BrowserField>,
+    raw_json: std::sync::Arc<JSONValue>,
 }
 
 impl PackageJson {
@@ -42,13 +34,10 @@ impl PackageJson {
         path: PathBuf,
         realpath: PathBuf,
         json: &str,
-        options: &ResolveOptions,
     ) -> Result<Self, serde_json::Error> {
         let mut raw_json: JSONValue = serde_json::from_str(json)?;
         let mut package_json = Self::default();
 
-        package_json.browser_fields.reserve_exact(options.alias_fields.len());
-
         if let Some(json_object) = raw_json.as_object_mut() {
             // Remove large fields that are useless for pragmatic use.
             #[cfg(feature = "package_json_raw_json_api")]
@@ -65,32 +54,6 @@ impl PackageJson {
             // Add name.
             package_json.name =
                 json_object.get("name").and_then(|field| field.as_str()).map(ToString::to_string);
-
-            // Dynamically create `browser_fields`.
-            let dir = path.parent().unwrap();
-            for object_path in &options.alias_fields {
-                if let Some(browser_field) = Self::get_value_by_path(json_object, object_path) {
-                    let mut browser_field = BrowserField::deserialize(browser_field)?;
-
-                    // Normalize all relative paths to make browser_field a constant value lookup
-                    if let BrowserField::Map(map) = &mut browser_field {
-                        let keys = map.keys().cloned().collect::<Vec<_>>();
-                        for key in keys {
-                            // Normalize the key if it looks like a file "foo.js"
-                            if key.extension().is_some() {
-                                map.insert(dir.normalize_with(&key), map[&key].clone());
-                            }
-                            // Normalize the key if it is relative path "./relative"
-                            if key.starts_with(".") {
-                                if let Some(value) = map.remove(&key) {
-                                    map.insert(dir.normalize_with(&key), value);
-                                }
-                            }
-                        }
-                    }
-                    package_json.browser_fields.push(browser_field);
-                }
-            }
         }
 
         package_json.path = path;
@@ -180,7 +143,7 @@ impl PackageJson {
     pub(crate) fn imports_fields<'a>(
         &'a self,
         imports_fields: &'a [Vec<String>],
-    ) -> impl Iterator<Item = &'a ImportExportMap> + '_ {
+    ) -> impl Iterator<Item = &'a JSONMap> + '_ {
         imports_fields.iter().filter_map(|object_path| {
             self.raw_json
                 .as_object()
@@ -189,27 +152,48 @@ impl PackageJson {
         })
     }
 
+    /// The "browser" field is provided by a module author as a hint to javascript bundlers or component tools when packaging modules for client side use.
+    /// Multiple values are configured by [ResolveOptions::alias_fields].
+    ///
+    /// <https://github.com/defunctzombie/package-browser-field-spec>
+    fn browser_fields<'a>(
+        &'a self,
+        alias_fields: &'a [Vec<String>],
+    ) -> impl Iterator<Item = &'a JSONMap> + '_ {
+        alias_fields.iter().filter_map(|object_path| {
+            self.raw_json
+                .as_object()
+                .and_then(|json_object| Self::get_value_by_path(json_object, object_path))
+                // Only object is valid, all other types are invalid
+                // https://github.com/webpack/enhanced-resolve/blob/3a28f47788de794d9da4d1702a3a583d8422cd48/lib/AliasFieldPlugin.js#L44-L52
+                .and_then(|value| value.as_object())
+        })
+    }
+
     /// Resolve the request string for this package.json by looking at the `browser` field.
     ///
     /// # Errors
     ///
     /// * Returns [ResolveError::Ignored] for `"path": false` in `browser` field.
-    pub(crate) fn resolve_browser_field(
-        &self,
+    pub(crate) fn resolve_browser_field<'a>(
+        &'a self,
         path: &Path,
         request: Option<&str>,
+        alias_fields: &'a [Vec<String>],
     ) -> Result<Option<&str>, ResolveError> {
-        if self.browser_fields.is_empty() {
-            return Ok(None);
-        }
-        let request = request.map_or(path, Path::new);
-        for browser in &self.browser_fields {
-            // Only object is valid, all other types are invalid
-            // https://github.com/webpack/enhanced-resolve/blob/3a28f47788de794d9da4d1702a3a583d8422cd48/lib/AliasFieldPlugin.js#L44-L52
-            if let BrowserField::Map(field_data) = browser {
-                if let Some(value) = field_data.get(request) {
+        for object in self.browser_fields(alias_fields) {
+            if let Some(request) = request {
+                if let Some(value) = object.get(request) {
                     return Self::alias_value(path, value);
                 }
+            } else {
+                let dir = self.path.parent().unwrap();
+                for (key, value) in object {
+                    let joined = dir.normalize_with(key);
+                    if joined == path {
+                        return Self::alias_value(path, value);
+                    }
+                }
             }
         }
         Ok(None)


Test patch:
diff --git a/src/tests/browser_field.rs b/src/tests/browser_field.rs
index 8855b58..60c21cd 100644
--- a/src/tests/browser_field.rs
+++ b/src/tests/browser_field.rs
@@ -30,6 +30,25 @@ fn ignore() {
     }
 }
 
+#[test]
+fn shared_resolvers() {
+    let f = super::fixture().join("browser-module");
+
+    let resolver1 = Resolver::new(ResolveOptions {
+        alias_fields: vec![vec!["innerBrowser1".into(), "field".into(), "browser".into()]],
+        ..ResolveOptions::default()
+    });
+    let resolved_path = resolver1.resolve(&f, "./lib/main1.js").map(|r| r.full_path());
+    assert_eq!(resolved_path, Ok(f.join("lib/main.js")));
+
+    let resolver2 = resolver1.clone_with_options(ResolveOptions {
+        alias_fields: vec![vec!["innerBrowser2".into(), "browser".into()]],
+        ..ResolveOptions::default()
+    });
+    let resolved_path = resolver2.resolve(&f, "./lib/main2.js").map(|r| r.full_path());
+    assert_eq!(resolved_path, Ok(f.join("./lib/replaced.js")));
+}
+
 #[test]
 fn replace_file() {
     let f = super::fixture().join("browser-module");
diff --git a/src/tests/imports_field.rs b/src/tests/imports_field.rs
index 3bf81eb..9f9a972 100644
--- a/src/tests/imports_field.rs
+++ b/src/tests/imports_field.rs
@@ -4,7 +4,7 @@
 
 use serde_json::json;
 
-use crate::{Ctx, ImportExportMap, PathUtil, ResolveError, ResolveOptions, Resolver};
+use crate::{Ctx, JSONMap, PathUtil, ResolveError, ResolveOptions, Resolver};
 use std::path::Path;
 
 #[test]
@@ -94,13 +94,13 @@ fn shared_resolvers() {
 struct TestCase {
     name: &'static str,
     expect: Option<Vec<&'static str>>,
-    imports_field: ImportExportMap,
+    imports_field: JSONMap,
     request: &'static str,
     condition_names: Vec<&'static str>,
 }
 
 #[allow(clippy::needless_pass_by_value)]
-fn imports_field(value: serde_json::Value) -> ImportExportMap {
+fn imports_field(value: serde_json::Value) -> JSONMap {
     let s = serde_json::to_string(&value).unwrap();
     serde_json::from_str(&s).unwrap()
 }