Follow up to #843, refining exports and browser field interaction

packages/node-resolve/README.md

@@ -34,9 +34,9 @@ export default {
   input: 'src/index.js',
   output: {
     dir: 'output',
-    format: 'cjs'
+    format: 'cjs',
   },
-  plugins: [nodeResolve()]
+  plugins: [nodeResolve()],
 };
 ```
 
@@ -64,7 +64,7 @@ Setting this option will add extra conditions on top of the default conditions.
 Type: `Boolean`<br>
 Default: `false`
 
-If `true`, instructs the plugin to use the `"browser"` property in `package.json` files to specify alternative files to load for bundling. This is useful when bundling for a browser environment. Alternatively, a value of `'browser'` can be added to the `mainFields` option. If `false`, any `"browser"` properties in package files will be ignored. This option takes precedence over `mainFields`.
+If `true`, instructs the plugin to use the browser module resolutions in `package.json` and adds `'browser'` to `exportConditions` if it is not present so browser conditionals in `exports` are applied. If `false`, any browser properties in package files will be ignored. Alternatively, a value of `'browser'` can be added to the `mainFields` option, however this option takes precedence over `mainFields`.
 
 > This option does not work when a package is using [package entrypoints](https://nodejs.org/api/packages.html#packages_package_entry_points)
 
@@ -179,9 +179,9 @@ export default {
   output: {
     file: 'bundle.js',
     format: 'iife',
-    name: 'MyModule'
+    name: 'MyModule',
   },
-  plugins: [nodeResolve(), commonjs()]
+  plugins: [nodeResolve(), commonjs()],
 };
 ```
 
@@ -216,7 +216,7 @@ The node resolve plugin uses `import` by default, you can opt into using the `re
 ```js
 this.resolve(importee, importer, {
   skipSelf: true,
-  custom: { 'node-resolve': { isRequire: true } }
+  custom: { 'node-resolve': { isRequire: true } },
 });
 ```
 

packages/node-resolve/src/index.js

@@ -22,6 +22,7 @@ const deepFreeze = (object) => {
     }
   }
 
+
   return object;
 };
 
@@ -161,6 +162,9 @@ export function nodeResolve(opts = {}) {
       opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
     const exportConditions = isRequire ? conditionsCjs : conditionsEsm;
 
+    if (useBrowserOverrides && !exportConditions.includes('browser'))
+      exportConditions.push('browser');
+
     const resolvedWithoutBuiltins = await resolveImportSpecifiers({
       importer,
       importSpecifierList,

packages/node-resolve/src/package/resolvePackageTarget.js

@@ -24,11 +24,11 @@ async function resolvePackageTarget(context, { target, subpath, pattern, interna
             target.replace(/\*/g, subpath),
             context.pkgURL.href
           );
-          return result ? pathToFileURL(result.location) : null;
+          return result ? pathToFileURL(result.location).href : null;
         }
 
         const result = await context.resolveId(`${target}${subpath}`, context.pkgURL.href);
-        return result ? pathToFileURL(result.location) : null;
+        return result ? pathToFileURL(result.location).href : null;
       }
       throw new InvalidPackageTargetError(context, `Invalid mapping: "${target}".`);
     }

packages/node-resolve/test/browser.js

@@ -199,3 +199,23 @@ test('pkg.browser with mapping to prevent bundle by specifying a value of false'
 
   t.is(module.exports, 'ok');
 });
+
+test('exports.browser can be mapped via pkg.browser', async (t) => {
+  const bundle = await rollup({
+    input: 'browser-exports-browser-browser.js',
+    plugins: [nodeResolve({ browser: true }), commonjs()]
+  });
+  const { module } = await testBundle(t, bundle);
+
+  t.is(module.exports, 'browser');
+});
+
+test('exports.browser does not take precedence over export map result, when browser:false', async (t) => {
+  const bundle = await rollup({
+    input: 'browser-exports-browser.js',
+    plugins: [nodeResolve({browser: true}), commonjs()]
+  });
+  const { module } = await testBundle(t, bundle);
+
+  t.is(module.exports, 'require');
+});

packages/node-resolve/test/fixtures/browser-exports-browser-browser.js

@@ -0,0 +1,3 @@
+import b from 'exports-browser-browser';
+
+module.exports = b;

packages/node-resolve/test/fixtures/browser-exports-browser.js

@@ -0,0 +1,3 @@
+const b = require('exports-browser');
+
+module.exports = b;

packages/node-resolve/test/fixtures/browser-override-exports.js

@@ -0,0 +1,3 @@
+const Nanoid = require('nanoid');
+
+module.exports = Nanoid;