feat(node-resolve): Follow up to #843, refining exports and browser field interaction (#866)

Author: Jarreddebeer

packages/node-resolve/README.md

@@ -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 both the `mainFields` and `exportConditions` options, 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)
 

packages/node-resolve/src/index.js

@@ -161,6 +161,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('browser field does not take precedence over export map result', 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;