Repository: oxc-project/oxc-resolver
Pull request #141 (2024-04-23 10:17:09)

refactor: remove the browser field lookup in `resolve_esm_match`

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/141?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 95.07%. Comparing base [(`c971213`)](https://app.codecov.io/gh/oxc-project/oxc-resolver/commit/c971213b62f0627dde6606e61deb96c51826a314?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oxc-project) to head [(`669aa5f`)](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/141?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     #141      +/-   ##
==========================================
- Coverage   95.08%   95.07%   -0.01%     
==========================================
  Files          11       11              
  Lines        2257     2253       -4     
==========================================
- Hits         2146     2142       -4     
  Misses        111      111              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/oxc-project/oxc-resolver/pull/141?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).

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

### Merging #141 will **not alter performance**

<sub>Comparing <code>browser-field</code> (669aa5f) with <code>main</code> (c971213)</sub>



### Summary

`✅ 2` untouched benchmarks






Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 1b8ac85..8372948 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -477,7 +477,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         // 4. let MATCH = PACKAGE_IMPORTS_RESOLVE(X, pathToFileURL(SCOPE), ["node", "require"]) defined in the ESM resolver.
         if let Some(path) = self.package_imports_resolve(specifier, &package_json, ctx)? {
             // 5. RESOLVE_ESM_MATCH(MATCH).
-            return self.resolve_esm_match(specifier, &path, &package_json, ctx);
+            return self.resolve_esm_match(specifier, &path, ctx);
         }
         Ok(None)
     }
@@ -756,11 +756,10 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                 cached_path.path(),
                 &format!(".{subpath}"),
                 exports,
-                &self.options.condition_names,
                 ctx,
             )? {
                 // 6. RESOLVE_ESM_MATCH(MATCH)
-                return self.resolve_esm_match(specifier, &path, &package_json, ctx);
+                return self.resolve_esm_match(specifier, &path, ctx);
             };
         }
         Ok(None)
@@ -793,15 +792,11 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             // Note: The subpath is not prepended with a dot on purpose
             // because `package_exports_resolve` matches subpath without the leading dot.
             for exports in package_json.exports_fields(&self.options.exports_fields) {
-                if let Some(cached_path) = self.package_exports_resolve(
-                    package_url,
-                    &format!(".{subpath}"),
-                    exports,
-                    &self.options.condition_names,
-                    ctx,
-                )? {
+                if let Some(cached_path) =
+                    self.package_exports_resolve(package_url, &format!(".{subpath}"), exports, ctx)?
+                {
                     // 6. RESOLVE_ESM_MATCH(MATCH)
-                    return self.resolve_esm_match(specifier, &cached_path, &package_json, ctx);
+                    return self.resolve_esm_match(specifier, &cached_path, ctx);
                 }
             }
         }
@@ -813,12 +808,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         &self,
         specifier: &str,
         cached_path: &CachedPath,
-        package_json: &PackageJson,
         ctx: &mut Ctx,
     ) -> ResolveResult {
-        if let Some(path) = self.load_browser_field(cached_path, None, package_json, ctx)? {
-            return Ok(Some(path));
-        }
         // 1. let RESOLVED_PATH = fileURLToPath(MATCH)
         // 2. If the file at RESOLVED_PATH exists, load RESOLVED_PATH as its extension format. STOP
         //
@@ -1143,7 +1134,6 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                                 cached_path.path(),
                                 &format!(".{subpath}"),
                                 exports,
-                                &self.options.condition_names,
                                 ctx,
                             )? {
                                 return Ok(Some(path));
@@ -1180,9 +1170,9 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         package_url: &Path,
         subpath: &str,
         exports: &JSONValue,
-        conditions: &[String],
         ctx: &mut Ctx,
     ) -> ResolveResult {
+        let conditions = &self.options.condition_names;
         // 1. If exports is an Object with both a key starting with "." and a key not starting with ".", throw an Invalid Package Configuration error.
         if let JSONValue::Object(map) = exports {
             let mut has_dot = false;


Test patch:
diff --git a/src/tests/exports_field.rs b/src/tests/exports_field.rs
index 3fa2fb7..2fcfb1f 100644
--- a/src/tests/exports_field.rs
+++ b/src/tests/exports_field.rs
@@ -2511,15 +2511,21 @@ fn test_cases() {
     ];
 
     for case in test_cases {
-        let resolved = Resolver::default()
-            .package_exports_resolve(
-                Path::new(""),
-                case.request,
-                &case.exports_field,
-                &case.condition_names.iter().map(ToString::to_string).collect::<Vec<_>>(),
-                &mut Ctx::default(),
-            )
-            .map(|p| p.map(|p| p.to_path_buf()));
+        let resolved = Resolver::new(ResolveOptions {
+            condition_names: case
+                .condition_names
+                .iter()
+                .map(ToString::to_string)
+                .collect::<Vec<_>>(),
+            ..ResolveOptions::default()
+        })
+        .package_exports_resolve(
+            Path::new(""),
+            case.request,
+            &case.exports_field,
+            &mut Ctx::default(),
+        )
+        .map(|p| p.map(|p| p.to_path_buf()));
         if let Some(expect) = case.expect {
             if expect.is_empty() {
                 assert!(