oxc-project · Boshen · Dec 8, 2023 · Dec 8, 2023
diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/a.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/a.js
diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/b.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/b.js
diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/c.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/c.js
diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/d.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/d.js
diff --git a/...es/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json b/...es/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json
diff --git a/src/lib.rs b/src/lib.rs
@@ -790,13 +790,16 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
         package_json: &PackageJson,
         ctx: &mut ResolveContext,
     ) -> ResolveState {
-        let Some(specifier) = package_json.resolve_browser_field(path, specifier)? else {
+        let Some(new_specifier) = package_json.resolve_browser_field(path, specifier)? else {
             return Ok(None);
         };
-        if ctx.resolving_alias.as_ref().is_some_and(|s| s == specifier) {
+        if specifier.is_some_and(|s| s == new_specifier) {
             return Ok(None);
         }
-        let specifier = Specifier::parse(specifier).map_err(ResolveError::Specifier)?;
+        if ctx.resolving_alias.as_ref().is_some_and(|s| s == new_specifier) {
+            return Err(ResolveError::Recursion);
+        }
+        let specifier = Specifier::parse(new_specifier).map_err(ResolveError::Specifier)?;
         ctx.with_query_fragment(specifier.query, specifier.fragment);
         ctx.with_resolving_alias(specifier.path().to_string());
         ctx.with_fully_specified(false);
@@ -845,7 +848,7 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
                         if let Some(path) = self.load_alias_value(
                             cached_path,
                             alias_key,
-                            new_specifier.path(), // pass in passed alias value
+                            new_specifier.path(), // pass in parsed alias value
                             specifier,
                             ctx,
                         )? {

diff --git a/src/package_json.rs b/src/package_json.rs
@@ -151,15 +151,17 @@ impl PackageJson {
                 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 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);
+                    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);
+                            }
                         }
                     }
                 }
@@ -224,7 +226,10 @@ impl PackageJson {
         path: &Path,
         request: Option<&str>,
     ) -> Result<Option<&str>, ResolveError> {
-        let request = request.map_or(path, |r| Path::new(r));
+        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

diff --git a/src/tests/browser_field.rs b/src/tests/browser_field.rs
@@ -106,3 +106,26 @@ fn crypto_js() {
     let resolved_path = resolver.resolve(f.join("crypto-js"), "crypto").map(|r| r.full_path());
     assert_eq!(resolved_path, Err(ResolveError::Ignored(f.join("crypto-js"))));
 }
+
+// https://github.com/webpack/webpack/blob/87660921808566ef3b8796f8df61bd79fc026108/test/cases/resolving/browser-field/index.js#L40-L43
+#[test]
+fn recursive() {
+    let f = super::fixture().join("browser-module");
+
+    let resolver = Resolver::new(ResolveOptions {
+        alias_fields: vec![vec!["browser".into()]],
+        ..ResolveOptions::default()
+    });
+
+    let data = [
+        ("should handle recursive file 1", f.clone(), "recursive-file/a"),
+        ("should handle recursive file 2", f.clone(), "recursive-file/b"),
+        ("should handle recursive file 3", f.clone(), "recursive-file/c"),
+        ("should handle recursive file 4", f.clone(), "recursive-file/d"),
+    ];
+
+    for (comment, path, request) in data {
+        let resolved_path = resolver.resolve(&path, request);
+        assert_eq!(resolved_path, Err(ResolveError::Recursion), "{comment} {path:?} {request}");
+    }
+}