close #2001, fix #2002: "browser" map edge case

Author: evanw

CHANGELOG.md

@@ -2,6 +2,41 @@
 
 ## Unreleased
 
+* Handle an additional `browser` map edge case ([#2001](https://github.com/evanw/esbuild/pull/2001), [#2002](https://github.com/evanw/esbuild/issues/2002))
+
+    There is a community convention around the `browser` field in `package.json` that allows remapping import paths within a package when the package is bundled for use within a browser. There isn't a rigorous definition of how it's supposed to work and every bundler implements it differently. The approach esbuild uses is to try to be "maximally compatible" in that if at least one bundler exhibits a particular behavior regarding the `browser` map that allows a mapping to work, then esbuild also attempts to make that work.
+
+    I have a collection of test cases for this going here: https://github.com/evanw/package-json-browser-tests. However, I was missing test coverage for the edge case where a package path import in a subdirectory of the package could potentially match a remapping. The "maximally compatible" approach means replicating bugs in Browserify's implementation of the feature where package paths are mistaken for relative paths and are still remapped. Here's a specific example of an edge case that's now handled:
+
+    ```js
+    // entry.js
+    require('pkg/sub')
+    ```
+
+    ```json
+    // node_modules/pkg/package.json:
+    {
+      "browser": {
+        "./sub": "./sub/foo.js",
+        "./sub/sub": "./sub/bar.js"
+      }
+    }
+    ```
+
+    ```js
+    // node_modules/pkg/sub/foo.js:
+    require('sub')
+    ```
+
+    ```js
+    // node_modules/pkg/sub/bar.js:
+    console.log('works')
+    ```
+
+    The import path `sub` in `require('sub')` is mistaken for a relative path by Browserify due to a bug in Browserify, so Browserify treats it as if it were `./sub` instead. This is a Browserify-specific behavior and currently doesn't happen in any other bundler (except for esbuild, which attempts to replicate Browserify's bug).
+
+    Previously esbuild was incorrectly resolving `./sub` relative to the top-level package directory instead of to the subdirectory in this case, which meant `./sub` was incorrectly matching `"./sub": "./sub/foo.js"` instead of `"./sub/sub": "./sub/bar.js"`. This has been fixed so esbuild can now emulate Browserify's bug correctly in this edge case.
+
 * Support for esbuild with Linux on RISC-V 64bit ([#1624](https://github.com/evanw/esbuild/pull/1624))
 
     With this release, esbuild now has a published binary executable for the RISC-V 64bit architecture in the [`platform-linux-riscv64`](https://www.npmjs.com/package/platform-linux-riscv64) npm package. This change was contributed by [@piggynl](https://github.com/piggynl).

internal/bundler/bundler_packagejson_test.go

@@ -805,6 +805,49 @@ func TestPackageJsonBrowserIndexNoExt(t *testing.T) {
 	})
 }
 
+// See https://github.com/evanw/esbuild/issues/2002
+func TestPackageJsonBrowserIssue2002A(t *testing.T) {
+	packagejson_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/src/entry.js": `require('pkg/sub')`,
+			"/Users/user/project/src/node_modules/pkg/package.json": `{
+				"browser": {
+					"./sub": "./sub/foo.js"
+				}
+			}`,
+			"/Users/user/project/src/node_modules/pkg/sub/foo.js":   `require('sub')`,
+			"/Users/user/project/src/node_modules/sub/package.json": `{ "main": "./bar" }`,
+			"/Users/user/project/src/node_modules/sub/bar.js":       `works()`,
+		},
+		entryPaths: []string{"/Users/user/project/src/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/Users/user/project/out.js",
+		},
+	})
+}
+
+func TestPackageJsonBrowserIssue2002B(t *testing.T) {
+	packagejson_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/src/entry.js": `require('pkg/sub')`,
+			"/Users/user/project/src/node_modules/pkg/package.json": `{
+				"browser": {
+					"./sub": "./sub/foo.js",
+					"./sub/sub": "./sub/bar.js"
+				}
+			}`,
+			"/Users/user/project/src/node_modules/pkg/sub/foo.js": `require('sub')`,
+			"/Users/user/project/src/node_modules/pkg/sub/bar.js": `works()`,
+		},
+		entryPaths: []string{"/Users/user/project/src/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/Users/user/project/out.js",
+		},
+	})
+}
+
 func TestPackageJsonDualPackageHazardImportOnly(t *testing.T) {
 	packagejson_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_packagejson.txt

@@ -31,6 +31,46 @@ console.log(node);
 console.log(browser2);
 console.log(browser2);
 
+================================================================================
+TestPackageJsonBrowserIssue2002A
+---------- /Users/user/project/out.js ----------
+// Users/user/project/src/node_modules/sub/bar.js
+var require_bar = __commonJS({
+  "Users/user/project/src/node_modules/sub/bar.js"() {
+    works();
+  }
+});
+
+// Users/user/project/src/node_modules/pkg/sub/foo.js
+var require_foo = __commonJS({
+  "Users/user/project/src/node_modules/pkg/sub/foo.js"() {
+    require_bar();
+  }
+});
+
+// Users/user/project/src/entry.js
+require_foo();
+
+================================================================================
+TestPackageJsonBrowserIssue2002B
+---------- /Users/user/project/out.js ----------
+// Users/user/project/src/node_modules/pkg/sub/bar.js
+var require_bar = __commonJS({
+  "Users/user/project/src/node_modules/pkg/sub/bar.js"() {
+    works();
+  }
+});
+
+// Users/user/project/src/node_modules/pkg/sub/foo.js
+var require_foo = __commonJS({
+  "Users/user/project/src/node_modules/pkg/sub/foo.js"() {
+    require_bar();
+  }
+});
+
+// Users/user/project/src/entry.js
+require_foo();
+
 ================================================================================
 TestPackageJsonBrowserMapAvoidMissing
 ---------- /Users/user/project/out.js ----------

internal/resolver/package_json.go

@@ -197,7 +197,16 @@ func (r resolverQuery) checkBrowserMap(resolveDirInfo *dirInfo, inputPath string
 				}
 			}
 			if isInSamePackage {
-				checkPath("./" + inputPath)
+				relativePathPrefix := "./"
+
+				// Use the relative path from the file containing the import path to the
+				// enclosing package.json file. This includes any subdirectories within the
+				// package if there are any.
+				if relPath, ok := r.fs.Rel(resolveDirInfo.enclosingBrowserScope.absPath, resolveDirInfo.absPath); ok && relPath != "." {
+					relativePathPrefix += strings.ReplaceAll(relPath, "\\", "/") + "/"
+				}
+
+				checkPath(relativePathPrefix + inputPath)
 			}
 		}
 	}

npm/esbuild/package.json

@@ -24,9 +24,9 @@
     "esbuild-linux-64": "0.14.20",
     "esbuild-linux-arm": "0.14.20",
     "esbuild-linux-arm64": "0.14.20",
-    "esbuild-linux-riscv64": "0.14.20",
     "esbuild-linux-mips64le": "0.14.20",
     "esbuild-linux-ppc64le": "0.14.20",
+    "esbuild-linux-riscv64": "0.14.20",
     "esbuild-linux-s390x": "0.14.20",
     "esbuild-netbsd-64": "0.14.20",
     "esbuild-openbsd-64": "0.14.20",