From 099608f28ba1af5b8f6f98ac5ab05261ad45b42f Mon Sep 17 00:00:00 2001
From: Jeff See <jeffsee.55@gmail.com>
Date: Thu, 11 Jul 2024 10:48:22 -0700
Subject: [PATCH] fix: Handle the scenario where a package.json#browser field
 could be `false` (#427)

We [recently shipped](https://github.com/vercel/nft/pull/424) support
for the `browser` field, but we discovered an edge case where instead of
mapping to another string, the value could be `false` (eg,
[`object-inspect`](https://github.com/inspect-js/object-inspect/blob/main/package.json#L82)),
which indicates that this module should be treated as an empty object
(`{}`) (evidenced by its usage
[here](https://github.com/inspect-js/object-inspect/blob/main/index.js#L68-L70)).

The code was previously erroring on the string's `startsWith` checks,
causing the entire package to be omitted from the file mapping result.
This ensures that if the value is `false` we'll continue on.

---------

Co-authored-by: Steven <steven@ceriously.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
---
 src/resolve-dependency.ts                           | 12 +++++++++++-
 test/unit/browser-remappings-false/.gitignore       |  2 ++
 test/unit/browser-remappings-false/input.js         |  2 ++
 .../node_modules/pkg/browser.js                     |  1 +
 .../node_modules/pkg/index.js                       |  2 ++
 .../node_modules/pkg/package.json                   |  7 +++++++
 test/unit/browser-remappings-false/output.js        |  7 +++++++
 test/unit/browser-remappings-false/test-opts.json   |  3 +++
 test/unit/browser-remappings-string/.gitignore      |  2 ++
 test/unit/browser-remappings-string/input.js        |  2 ++
 .../node_modules/pkg/browser.js                     |  1 +
 .../node_modules/pkg/index.js                       |  2 ++
 .../node_modules/pkg/package.json                   |  5 +++++
 test/unit/browser-remappings-string/output.js       |  7 +++++++
 test/unit/browser-remappings-string/test-opts.json  |  3 +++
 test/unit/browser-remappings-undefined/.gitignore   |  2 ++
 test/unit/browser-remappings-undefined/input.js     |  2 ++
 .../node_modules/pkg/index.js                       |  1 +
 .../node_modules/pkg/package.json                   | 13 +++++++++++++
 .../node_modules/pkg/require-main.cjs               |  2 ++
 .../node_modules/pkg/subdir/import-main-browser.js  |  1 +
 .../node_modules/pkg/subdir/import-main.js          |  1 +
 .../node_modules/pkg/subdir/package.json            |  1 +
 test/unit/browser-remappings-undefined/output.js    |  9 +++++++++
 .../browser-remappings-undefined/test-opts.json     |  3 +++
 test/unit/browser-remappings/output.js              |  2 +-
 26 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 test/unit/browser-remappings-false/.gitignore
 create mode 100644 test/unit/browser-remappings-false/input.js
 create mode 100644 test/unit/browser-remappings-false/node_modules/pkg/browser.js
 create mode 100644 test/unit/browser-remappings-false/node_modules/pkg/index.js
 create mode 100644 test/unit/browser-remappings-false/node_modules/pkg/package.json
 create mode 100644 test/unit/browser-remappings-false/output.js
 create mode 100644 test/unit/browser-remappings-false/test-opts.json
 create mode 100644 test/unit/browser-remappings-string/.gitignore
 create mode 100644 test/unit/browser-remappings-string/input.js
 create mode 100644 test/unit/browser-remappings-string/node_modules/pkg/browser.js
 create mode 100644 test/unit/browser-remappings-string/node_modules/pkg/index.js
 create mode 100644 test/unit/browser-remappings-string/node_modules/pkg/package.json
 create mode 100644 test/unit/browser-remappings-string/output.js
 create mode 100644 test/unit/browser-remappings-string/test-opts.json
 create mode 100644 test/unit/browser-remappings-undefined/.gitignore
 create mode 100644 test/unit/browser-remappings-undefined/input.js
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/index.js
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/package.json
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main-browser.js
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js
 create mode 100644 test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json
 create mode 100644 test/unit/browser-remappings-undefined/output.js
 create mode 100644 test/unit/browser-remappings-undefined/test-opts.json

diff --git a/src/resolve-dependency.ts b/src/resolve-dependency.ts
index 826e86c7..3695f5a0 100644
--- a/src/resolve-dependency.ts
+++ b/src/resolve-dependency.ts
@@ -128,7 +128,7 @@ interface PkgCfg {
   main: string | undefined;
   exports: PackageTarget;
   imports: { [key: string]: PackageTarget };
-  browser?: string | { [key: string]: string };
+  browser?: unknown;
 }
 
 async function getPkgCfg(
@@ -259,8 +259,18 @@ async function resolveRemappings(
 ): Promise<void> {
   if (job.conditions?.includes('browser')) {
     const { browser: pkgBrowser } = pkgCfg;
+    if (!pkgBrowser) {
+      return;
+    }
     if (typeof pkgBrowser === 'object') {
       for (const [key, value] of Object.entries(pkgBrowser)) {
+        if (typeof value !== 'string') {
+          /**
+           * `false` can be used to specify that a file is not meant to be included.
+           * Downstream processing is expected to handle this case, and it should remain in the mapping result
+           */
+          continue;
+        }
         if (!key.startsWith('./') || !value.startsWith('./')) {
           continue;
         }
diff --git a/test/unit/browser-remappings-false/.gitignore b/test/unit/browser-remappings-false/.gitignore
new file mode 100644
index 00000000..a9412d2d
--- /dev/null
+++ b/test/unit/browser-remappings-false/.gitignore
@@ -0,0 +1,2 @@
+# include node_modules for testing
+!node_modules 
diff --git a/test/unit/browser-remappings-false/input.js b/test/unit/browser-remappings-false/input.js
new file mode 100644
index 00000000..76b86cf7
--- /dev/null
+++ b/test/unit/browser-remappings-false/input.js
@@ -0,0 +1,2 @@
+require('pkg');
+
diff --git a/test/unit/browser-remappings-false/node_modules/pkg/browser.js b/test/unit/browser-remappings-false/node_modules/pkg/browser.js
new file mode 100644
index 00000000..a5ceac31
--- /dev/null
+++ b/test/unit/browser-remappings-false/node_modules/pkg/browser.js
@@ -0,0 +1 @@
+module.exports = 'browser code';
diff --git a/test/unit/browser-remappings-false/node_modules/pkg/index.js b/test/unit/browser-remappings-false/node_modules/pkg/index.js
new file mode 100644
index 00000000..da058b11
--- /dev/null
+++ b/test/unit/browser-remappings-false/node_modules/pkg/index.js
@@ -0,0 +1,2 @@
+module.exports = 'legacy index';
+require("./browser")
diff --git a/test/unit/browser-remappings-false/node_modules/pkg/package.json b/test/unit/browser-remappings-false/node_modules/pkg/package.json
new file mode 100644
index 00000000..33d8de58
--- /dev/null
+++ b/test/unit/browser-remappings-false/node_modules/pkg/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "pkg",
+  "main": "index.js",
+  "browser": {
+    "./browser.js": false
+  }
+}
diff --git a/test/unit/browser-remappings-false/output.js b/test/unit/browser-remappings-false/output.js
new file mode 100644
index 00000000..bc5f3405
--- /dev/null
+++ b/test/unit/browser-remappings-false/output.js
@@ -0,0 +1,7 @@
+[
+  "package.json",
+  "test/unit/browser-remappings-false/input.js",
+  "test/unit/browser-remappings-false/node_modules/pkg/browser.js",
+  "test/unit/browser-remappings-false/node_modules/pkg/index.js",
+  "test/unit/browser-remappings-false/node_modules/pkg/package.json"
+]
diff --git a/test/unit/browser-remappings-false/test-opts.json b/test/unit/browser-remappings-false/test-opts.json
new file mode 100644
index 00000000..9e01628e
--- /dev/null
+++ b/test/unit/browser-remappings-false/test-opts.json
@@ -0,0 +1,3 @@
+{
+  "conditions": ["browser"]
+}
diff --git a/test/unit/browser-remappings-string/.gitignore b/test/unit/browser-remappings-string/.gitignore
new file mode 100644
index 00000000..a9412d2d
--- /dev/null
+++ b/test/unit/browser-remappings-string/.gitignore
@@ -0,0 +1,2 @@
+# include node_modules for testing
+!node_modules 
diff --git a/test/unit/browser-remappings-string/input.js b/test/unit/browser-remappings-string/input.js
new file mode 100644
index 00000000..76b86cf7
--- /dev/null
+++ b/test/unit/browser-remappings-string/input.js
@@ -0,0 +1,2 @@
+require('pkg');
+
diff --git a/test/unit/browser-remappings-string/node_modules/pkg/browser.js b/test/unit/browser-remappings-string/node_modules/pkg/browser.js
new file mode 100644
index 00000000..a5ceac31
--- /dev/null
+++ b/test/unit/browser-remappings-string/node_modules/pkg/browser.js
@@ -0,0 +1 @@
+module.exports = 'browser code';
diff --git a/test/unit/browser-remappings-string/node_modules/pkg/index.js b/test/unit/browser-remappings-string/node_modules/pkg/index.js
new file mode 100644
index 00000000..41deffde
--- /dev/null
+++ b/test/unit/browser-remappings-string/node_modules/pkg/index.js
@@ -0,0 +1,2 @@
+module.exports = 'legacy index';
+require("pkg/browser")
diff --git a/test/unit/browser-remappings-string/node_modules/pkg/package.json b/test/unit/browser-remappings-string/node_modules/pkg/package.json
new file mode 100644
index 00000000..5300cb04
--- /dev/null
+++ b/test/unit/browser-remappings-string/node_modules/pkg/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "pkg",
+  "main": "index.js",
+  "browser": "./browser.js"
+}
diff --git a/test/unit/browser-remappings-string/output.js b/test/unit/browser-remappings-string/output.js
new file mode 100644
index 00000000..1f3ecac2
--- /dev/null
+++ b/test/unit/browser-remappings-string/output.js
@@ -0,0 +1,7 @@
+[
+  "package.json",
+  "test/unit/browser-remappings-string/input.js",
+  "test/unit/browser-remappings-string/node_modules/pkg/browser.js",
+  "test/unit/browser-remappings-string/node_modules/pkg/index.js",
+  "test/unit/browser-remappings-string/node_modules/pkg/package.json"
+]
diff --git a/test/unit/browser-remappings-string/test-opts.json b/test/unit/browser-remappings-string/test-opts.json
new file mode 100644
index 00000000..9e01628e
--- /dev/null
+++ b/test/unit/browser-remappings-string/test-opts.json
@@ -0,0 +1,3 @@
+{
+  "conditions": ["browser"]
+}
diff --git a/test/unit/browser-remappings-undefined/.gitignore b/test/unit/browser-remappings-undefined/.gitignore
new file mode 100644
index 00000000..a9412d2d
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/.gitignore
@@ -0,0 +1,2 @@
+# include node_modules for testing
+!node_modules 
diff --git a/test/unit/browser-remappings-undefined/input.js b/test/unit/browser-remappings-undefined/input.js
new file mode 100644
index 00000000..76b86cf7
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/input.js
@@ -0,0 +1,2 @@
+require('pkg');
+
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/index.js b/test/unit/browser-remappings-undefined/node_modules/pkg/index.js
new file mode 100644
index 00000000..914a7c58
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/index.js
@@ -0,0 +1 @@
+module.exports = 'legacy index';
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/package.json b/test/unit/browser-remappings-undefined/node_modules/pkg/package.json
new file mode 100644
index 00000000..a43212cb
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "pkg",
+  "main": "index.js",
+  "exports": {
+    ".": {
+      "import": "./subdir/import-main.js",
+      "require": "./require-main.cjs"
+    },
+    "./asdf": {
+      "import": "./subdir/import-main.js"
+    }
+  }
+}
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs b/test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs
new file mode 100644
index 00000000..43b69844
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs
@@ -0,0 +1,2 @@
+import('pkg/asdf');
+module.exports = 'require main';
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main-browser.js b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main-browser.js
new file mode 100644
index 00000000..72b9016b
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main-browser.js
@@ -0,0 +1 @@
+export default 'import main browser';
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js
new file mode 100644
index 00000000..95bc9efa
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js
@@ -0,0 +1 @@
+export default 'import main';
diff --git a/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json
new file mode 100644
index 00000000..6990891f
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json
@@ -0,0 +1 @@
+{"type": "module"}
diff --git a/test/unit/browser-remappings-undefined/output.js b/test/unit/browser-remappings-undefined/output.js
new file mode 100644
index 00000000..f94da2b5
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/output.js
@@ -0,0 +1,9 @@
+[
+  "package.json",
+  "test/unit/browser-remappings-undefined/input.js",
+  "test/unit/browser-remappings-undefined/node_modules/pkg/index.js",
+  "test/unit/browser-remappings-undefined/node_modules/pkg/package.json",
+  "test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs",
+  "test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js",
+  "test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json"
+]
diff --git a/test/unit/browser-remappings-undefined/test-opts.json b/test/unit/browser-remappings-undefined/test-opts.json
new file mode 100644
index 00000000..9e01628e
--- /dev/null
+++ b/test/unit/browser-remappings-undefined/test-opts.json
@@ -0,0 +1,3 @@
+{
+  "conditions": ["browser"]
+}
diff --git a/test/unit/browser-remappings/output.js b/test/unit/browser-remappings/output.js
index 9864d640..546aa4a8 100644
--- a/test/unit/browser-remappings/output.js
+++ b/test/unit/browser-remappings/output.js
@@ -7,4 +7,4 @@
   "test/unit/browser-remappings/node_modules/pkg/subdir/import-main-browser.js",
   "test/unit/browser-remappings/node_modules/pkg/subdir/import-main.js",
   "test/unit/browser-remappings/node_modules/pkg/subdir/package.json"
-]
\ No newline at end of file
+]