From ce3ec6c4dd2f2d637e00d6f7000ed56d54206f57 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Fri, 29 May 2020 22:40:15 -0400
Subject: [PATCH] fix: remove support for browser fields.

BREAKING CHANGE: support for resolving `browser` field has been removed.

  The `browser` field has very inconsitent usage across the ecosystem
  and is often used in a way that conflicts with ES-module-first tooling
  (e.g. firebase/app points browser to cjs build).
---
 playground/TestModuleResolve.vue              |  7 +--
 playground/package.json                       |  1 -
 .../resolve-browser-field/out/cjs.node.js     |  1 -
 .../resolve-browser-field/out/esm.browser.js  |  1 -
 playground/resolve-browser-field/package.json |  9 ----
 src/node/build/index.ts                       |  3 +-
 src/node/depOptimizer.ts                      | 15 ++++--
 src/node/resolver.ts                          | 52 ++-----------------
 test/test.js                                  |  1 -
 9 files changed, 17 insertions(+), 73 deletions(-)
 delete mode 100644 playground/resolve-browser-field/out/cjs.node.js
 delete mode 100644 playground/resolve-browser-field/out/esm.browser.js
 delete mode 100644 playground/resolve-browser-field/package.json

diff --git a/playground/TestModuleResolve.vue b/playground/TestModuleResolve.vue
index cb33a453f4fe30..281701ea6ce804 100644
--- a/playground/TestModuleResolve.vue
+++ b/playground/TestModuleResolve.vue
@@ -16,9 +16,6 @@
   <div class="dot-resolve" :class="dotResolve">
     filename with dot resolve: {{ dotResolve }}
   </div>
-  <div class="browser-field-resolve" :class="browserFieldResolve">
-    resolve browser field in package.json: {{ browserFieldResolve }}
-  </div>
 </template>
 
 <script>
@@ -28,7 +25,6 @@ import { add } from 'lodash-es'
 import { test } from 'conditional-exports'
 import { foo } from './util'
 import { bar } from './util/bar.util'
-import value from 'resolve-browser-field-test-package'
 
 export default {
   setup() {
@@ -38,8 +34,7 @@ export default {
       optResolve: typeof add === 'function' ? 'ok' : 'error',
       conditionalExports: test() ? 'ok' : 'error',
       indexResolve: foo() ? 'ok' : 'error',
-      dotResolve: bar() ? 'ok' : 'error',
-      browserFieldResolve: value === 'success' ? 'ok' : 'error'
+      dotResolve: bar() ? 'ok' : 'error'
     }
   }
 }
diff --git a/playground/package.json b/playground/package.json
index e0891b3ae52f2c..64cb1cc0e8a599 100644
--- a/playground/package.json
+++ b/playground/package.json
@@ -9,7 +9,6 @@
     "conditional-exports": "link:./conditional-exports",
     "lodash-es": "link:../node_modules/lodash-es",
     "moment": "link:../node_modules/moment",
-    "resolve-browser-field-test-package": "link:./resolve-browser-field",
     "rewrite-optimized-test-package": "link:./rewrite-optimized/test-package"
   }
 }
diff --git a/playground/resolve-browser-field/out/cjs.node.js b/playground/resolve-browser-field/out/cjs.node.js
deleted file mode 100644
index 95090a641f78b4..00000000000000
--- a/playground/resolve-browser-field/out/cjs.node.js
+++ /dev/null
@@ -1 +0,0 @@
-export default 'fail'
diff --git a/playground/resolve-browser-field/out/esm.browser.js b/playground/resolve-browser-field/out/esm.browser.js
deleted file mode 100644
index 739bb26e01b765..00000000000000
--- a/playground/resolve-browser-field/out/esm.browser.js
+++ /dev/null
@@ -1 +0,0 @@
-export default 'success'
diff --git a/playground/resolve-browser-field/package.json b/playground/resolve-browser-field/package.json
deleted file mode 100644
index 12aa76fc87ccdc..00000000000000
--- a/playground/resolve-browser-field/package.json
+++ /dev/null
@@ -1,9 +0,0 @@
-{
-  "name": "resolve-browser-field-test-package",
-  "version": "1.0.0",
-  "//": "real world example: https://github.com/axios/axios/blob/3f2ef030e001547eb06060499f8a2e3f002b5a14/package.json#L71-L73",
-  "main": "out/cjs.node.js",
-  "browser": {
-    "./out/cjs.node.js": "./out/esm.browser.js"
-  }
-}
diff --git a/src/node/build/index.ts b/src/node/build/index.ts
index ffd4762d10b550..9e1eaaddc89def 100644
--- a/src/node/build/index.ts
+++ b/src/node/build/index.ts
@@ -115,8 +115,7 @@ export async function createBaseRollupPlugins(
       extensions: supportedExts,
       preferBuiltins: false,
       dedupe: options.rollupDedupe || [],
-      mainFields,
-      browser: true
+      mainFields
     }),
     require('@rollup/plugin-commonjs')({
       extensions: ['.js', '.cjs']
diff --git a/src/node/depOptimizer.ts b/src/node/depOptimizer.ts
index 6f573b8c66a49f..74dc70fc14ec3d 100644
--- a/src/node/depOptimizer.ts
+++ b/src/node/depOptimizer.ts
@@ -258,14 +258,19 @@ export async function optimizeDeps(
       console.log()
       console.log(
         chalk.yellow(
-          `Tip: Make sure your "dependencies" only include packages that you ` +
-            `intend to use in the browser. If it's a Node.js package, it ` +
-            `should be in "devDependencies". You can also configure what deps ` +
-            `to include/exclude for optimization using the \`optimizeDeps\` ` +
-            `option in the vite config file.`
+          `Tip:\nMake sure your "dependencies" only include packages that you\n` +
+            `intend to use in the browser. If it's a Node.js package, it\n` +
+            `should be in "devDependencies".\n\n` +
+            `You can also configure what deps to include/exclude for\n` +
+            `optimization using the "optimizeDeps" option in vite.config.js.\n\n` +
+            `If you do intend to use this dependency in the browser, then\n` +
+            `unfortunately it is not distributed in a web-friendly way and\n` +
+            `it is recommended to look for a modern alternative that ships\n` +
+            `ES module build.\n`
         )
         // TODO link to docs once we have it
       )
+      process.exit(1)
     }
   }
 }
diff --git a/src/node/resolver.ts b/src/node/resolver.ts
index 6b8d2498c8b0e3..cb0580c5de93f8 100644
--- a/src/node/resolver.ts
+++ b/src/node/resolver.ts
@@ -332,9 +332,8 @@ export function resolveNodeModule(
       }
     }
 
-    let mainField
     if (!entryPoint) {
-      for (mainField of mainFields) {
+      for (const mainField of mainFields) {
         if (pkg[mainField]) {
           entryPoint = pkg[mainField]
           break
@@ -342,29 +341,6 @@ export function resolveNodeModule(
       }
     }
 
-    // resolve browser field in package.json
-    // https://github.com/defunctzombie/package-browser-field-spec
-    const browserField = pkg.browser
-    if (typeof browserField === 'string' && mainField === 'main') {
-      // Only overwrite already resolved if we did not find a ESM compatible
-      // entry. This is because some packages e.g. firebase/app assumes
-      // "module" is resolved with higher priority than "browser" and its
-      // "browser" build actually points to a CommonJS file.
-      // Note this does cause the build to resolve to a different file in this
-      // case because @rollup/plugin-node-resolve treats "browser" with highest
-      // priority; but as long as a lib's esm build can run in the browser, its
-      // behavior should be exactly the same as the "browser" build.
-      entryPoint = browserField
-    } else if (
-      entryPoint &&
-      typeof browserField === 'object' &&
-      browserField !== null
-    ) {
-      entryPoint = mapWithBrowserField(entryPoint, browserField)
-    }
-
-    debug(`(node_module entry) ${id} -> ${entryPoint}`)
-
     // save resolved entry file path using the deep import path as key
     // e.g. foo/dist/foo.js
     // this is the path raw imports will be rewritten to, and is what will
@@ -384,6 +360,10 @@ export function resolveNodeModule(
       nodeModulesFileMap.set(entryPoint, entryFilePath)
     }
 
+    if (entryPoint) {
+      debug(`(node_module entry) ${id} -> ${entryPoint}`)
+    }
+
     const result: NodeModuleInfo = {
       entry: entryPoint!,
       entryFilePath,
@@ -410,25 +390,3 @@ export function resolveNodeModuleFile(
     // error will be reported downstream
   }
 }
-
-const normalize = path.posix.normalize
-
-/**
- * given a relative path in pkg dir,
- * return a relative path in pkg dir,
- * mapped with the "map" object
- */
-function mapWithBrowserField(
-  relativePathInPkgDir: string,
-  map: Record<string, string>
-) {
-  const normalized = normalize(relativePathInPkgDir)
-  const foundEntry = Object.entries(map).find(([from]) => {
-    return normalize(from) === normalized
-  })
-  if (!foundEntry) {
-    return normalized
-  }
-  const [, to] = foundEntry
-  return normalize(to)
-}
diff --git a/test/test.js b/test/test.js
index 7f5f335199730a..4a77124fc90204 100644
--- a/test/test.js
+++ b/test/test.js
@@ -108,7 +108,6 @@ describe('vite', () => {
       expect(await getText('.module-resolve-conditional')).toMatch('ok')
       expect(await getText('.index-resolve')).toMatch('ok')
       expect(await getText('.dot-resolve')).toMatch('ok')
-      expect(await getText('.browser-field-resolve')).toMatch('ok')
     })
 
     if (!isBuild) {