feat(resolver): complete browser_field implementation

crates/oxc_resolver/src/lib.rs

@@ -78,6 +78,7 @@ impl ResolveContext {
             fully_specified: false,
             query: ctx.query.clone(),
             fragment: ctx.fragment.clone(),
+            resolving_alias: ctx.resolving_alias.clone(),
             depth: ctx.depth,
         }))
     }
@@ -92,6 +93,10 @@ impl ResolveContext {
         self.borrow_mut().fragment = fragment.map(ToString::to_string);
     }
 
+    fn with_resolving_alias(&self, alias: String) {
+        self.borrow_mut().resolving_alias = Some(alias);
+    }
+
     fn test_for_infinite_recursion(&self) -> Result<(), ResolveError> {
         self.borrow_mut().depth += 1;
         // 64 should be more than enough for detecting infinite recursion.
@@ -107,6 +112,8 @@ struct ResolveContextImpl {
     fully_specified: bool,
     query: Option<String>,
     fragment: Option<String>,
+    /// The current resolving alias for bailing recursion alias.
+    resolving_alias: Option<String>,
     /// For avoiding infinite recursion, which will cause stack overflow.
     depth: u8,
 }
@@ -626,12 +633,13 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         package_json: &PackageJson,
         ctx: &ResolveContext,
     ) -> ResolveState {
-        if !self.options.alias_fields.iter().any(|field| field == "browser") {
-            return Ok(None);
-        }
         let Some(specifier) = package_json.resolve_browser_field(path, specifier)? else {
             return Ok(None);
         };
+        if ctx.borrow().resolving_alias.as_ref().is_some_and(|s| s == specifier) {
+            return Ok(None);
+        }
+        ctx.with_resolving_alias(specifier.to_string());
         let cached_path = self.cache.value(package_json.directory());
         let ctx = ResolveContext::clone_from(ctx);
         self.require(&cached_path, specifier, &ctx).map(Some)

crates/oxc_resolver/src/package_json.rs

@@ -49,9 +49,11 @@ pub struct PackageJson {
     pub imports: Box<MatchObject>,
 
     /// 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: Option<BrowserField>,
+    #[serde(skip)]
+    pub browser_fields: Vec<BrowserField>,
 }
 
 /// `matchObj` defined in `PACKAGE_IMPORTS_EXPORTS_RESOLVE`
@@ -113,6 +115,8 @@ pub enum BrowserField {
 }
 
 impl PackageJson {
+    /// # Panics
+    /// # Errors
     pub fn parse(
         path: PathBuf,
         json: &str,
@@ -120,14 +124,40 @@ impl PackageJson {
     ) -> Result<Self, serde_json::Error> {
         let mut package_json_value: serde_json::Value = serde_json::from_str(json.clone())?;
 
-        // Dynamically create `main_fields`.
-        let mut main_fields = vec![];
+        let mut main_fields = Vec::with_capacity(options.main_fields.len());
+        let mut browser_fields = Vec::with_capacity(options.alias_fields.len());
+
         if let Some(package_json_value) = package_json_value.as_object_mut() {
+            // Dynamically create `main_fields`.
             for main_field_key in &options.main_fields {
+                // Using `get` + `clone` instead of remove here
+                // because `main_fields` may contain `browser`, which is also used in `browser_fields.
                 if let Some(serde_json::Value::String(value)) =
-                    package_json_value.remove(main_field_key)
+                    package_json_value.get(main_field_key)
                 {
-                    main_fields.push(value);
+                    main_fields.push(value.clone());
+                }
+            }
+            // Dynamically create `browser_fields`.
+            let dir = path.parent().unwrap();
+            for browser_field_key in &options.alias_fields {
+                if let Some(value) = package_json_value.remove(browser_field_key) {
+                    let mut browser_field: BrowserField = serde_json::from_value(value)?;
+                    // Normalize all relative paths to make browser_field a constant value lookup
+                    if let BrowserField::Map(map) = &mut browser_field {
+                        let relative_paths = map
+                            .keys()
+                            .filter(|path| path.starts_with("."))
+                            .cloned()
+                            .collect::<Vec<_>>();
+                        for relative_path in relative_paths {
+                            if let Some(value) = map.remove(&relative_path) {
+                                let normalized_path = dir.normalize_with(relative_path);
+                                map.insert(normalized_path, value);
+                            }
+                        }
+                    }
+                    browser_fields.push(browser_field);
                 }
             }
         }
@@ -135,20 +165,7 @@ impl PackageJson {
         // TODO: can this clone be avoided?
         let mut package_json: Self = serde_json::from_str(json.clone())?;
         package_json.main_fields = main_fields;
-
-        // Normalize all relative paths to make browser_field a constant value lookup
-        // TODO: fix BrowserField::String
-        if let Some(BrowserField::Map(map)) = &mut package_json.browser {
-            let relative_paths =
-                map.keys().filter(|path| path.starts_with(".")).cloned().collect::<Vec<_>>();
-            let dir = path.parent().unwrap();
-            for relative_path in relative_paths {
-                if let Some(value) = map.remove(&relative_path) {
-                    let normalized_path = dir.normalize_with(relative_path);
-                    map.insert(normalized_path, value);
-                }
-            }
-        }
+        package_json.browser_fields = browser_fields;
 
         package_json.path = path;
         Ok(package_json)
@@ -170,20 +187,21 @@ impl PackageJson {
         path: &Path,
         request: Option<&str>,
     ) -> Result<Option<&str>, ResolveError> {
-        // TODO: return ResolveError if the provided `alias_fields` is not `browser` for future proof
-        match self.browser.as_ref() {
-            Some(BrowserField::Map(field_data)) => {
-                // look up by full path if request is empty
-                request
-                    .map_or_else(
-                        || field_data.get(path),
-                        |request| field_data.get(Path::new(request)),
-                    )
-                    .map_or_else(|| Ok(None), |value| Self::alias_value(path, value))
+        let request = request.map_or(path, |r| Path::new(r));
+        for browser in &self.browser_fields {
+            match browser {
+                BrowserField::Map(field_data) => {
+                    // look up by full path if request is empty
+                    if let Some(value) = field_data.get(request) {
+                        return Self::alias_value(path, value);
+                    }
+                }
+                BrowserField::String(value) => {
+                    return Ok(Some(value.as_str()));
+                }
             }
-            // TODO: implement <https://github.com/defunctzombie/package-browser-field-spec#alternate-main---basic>
-            _ => Ok(None),
         }
+        Ok(None)
     }
 
     fn alias_value<'a>(

crates/oxc_resolver/src/tests/browser_field.rs

@@ -32,6 +32,8 @@ fn replace_file() {
 
     let resolver = Resolver::new(ResolveOptions {
         alias_fields: vec!["browser".into()],
+        // Not part of enhanced-resolve. Added to make sure no interaction between these two fields.
+        main_fields: vec!["browser".into()],
         ..ResolveOptions::default()
     });
 
@@ -50,6 +52,8 @@ fn replace_file() {
         // TODO: resolve `innerBrowser2` field in `browser-module/pakckage.json`
         // ("should resolve in nested property 2", f.clone(), "./lib/main2.js", f.join("lib/browser.js")),
         ("should check only alias field properties", f.clone(), "./toString", f.join("lib/toString.js")),
+        // not part of enhanced-resolve
+        ("recursion", f.clone(), "module-c", f.join("node_modules/module-c.js")),
     ];
 
     for (comment, path, request, expected) in data {

crates/oxc_resolver/tests/enhanced_resolve/test/fixtures/browser-module/package.json

@@ -4,6 +4,7 @@
     "./lib/replaced.js": "./lib/browser",
     "module-a": "./browser/module-a.js",
     "module-b": "module-c",
+    "module-c": "module-c",
     "./toString": "./lib/toString.js",
     ".": false
   },