Merge pull request #389 from preactjs/default-over-node

(wmr) - resolve default over node

Author: marvinhagemeister

.changeset/strange-beers-play.md

@@ -0,0 +1,5 @@
+---
+'wmr': minor
+---
+
+Use resolve.exports for export map resolving in the npm-plugin

.gitignore

@@ -6,4 +6,5 @@ wmr.cjs
 package-lock.json
 !packages/wmr/test/fixtures/commonjs/node_modules
 !packages/wmr/test/fixtures/package-exports/node_modules
+!packages/wmr/test/fixtures/exports/node_modules
 .DS_Store

packages/wmr/package.json

@@ -139,6 +139,7 @@
 		"premove": "^3.0.1",
 		"prettier": "^2.0.5",
 		"puppeteer": "^3.3.0",
+		"resolve.exports": "^1.0.2",
 		"rollup": "^2.39.0",
 		"rollup-plugin-preserve-shebang": "^1.0.1",
 		"sade": "^1.7.3",

packages/wmr/src/plugins/npm-plugin/resolve.js

@@ -1,3 +1,21 @@
+import { resolve as _resolveExports, legacy as _resolveLegacyEntry } from 'resolve.exports';
+
+function resolveExports(pkg, key) {
+	return _resolveExports(pkg, key, {
+		browser: true,
+		conditions: [process.env.NODE_ENV === 'production' ? 'production' : 'development', 'esmodules', 'module']
+	});
+}
+
+function resolveLegacyEntry(pkg, path) {
+	const entry =
+		_resolveLegacyEntry(pkg, {
+			browser: true,
+			fields: ['esmodules', 'modern', 'module', 'jsnext:main', 'browser', 'main']
+		}) || 'index.js';
+	return '/' + entry.replace(/^\.?\//, '');
+}
+
 /**
  * @param {string} path
  * @param {object} context
@@ -21,23 +39,16 @@ export async function resolveModule(path, { readFile, hasFile, module, internal
 
 	// Package Export Maps
 	if (!internal && pkg.exports) {
-		const entry = path ? `./${path}` : '.';
-
-		const mapped = resolveExportMap(pkg.exports, entry, ENV_KEYS);
-
-		if (!mapped) {
-			throw new Error(`Unknown package export ${entry} in ${module}.\n\n${JSON.stringify(pkg.exports, null, 2)}`);
-		}
-
-		// `mapped:true` means directory access was allowed for this entry, but it was not resolved.
-		if (mapped !== true && !internal) {
-			return mapped.replace(/^\./, '');
-		}
+		// will normalize entry & will throw error if no match
+		const mapped = resolveExports(pkg, path || '.');
+		// An entry ending in `/` remaps to a directory, but is not considered resolved.
+		if (mapped.endsWith('/')) path = mapped;
+		else return mapped.replace(/^\./, '');
 	}
 
 	// path is a bare import of a package, use its legacy exports (module/main):
 	if (!path) {
-		path = getLegacyEntry(pkg);
+		path = resolveLegacyEntry(pkg, path || '.');
 	}
 
 	// fallback: implement basic commonjs-style resolution
@@ -50,7 +61,7 @@ export async function resolveModule(path, { readFile, hasFile, module, internal
 	if (!isExportMappedSpecifier) {
 		try {
 			const subPkg = JSON.parse(await readFile(path + '/package.json'));
-			path += getLegacyEntry(subPkg);
+			path += resolveLegacyEntry(subPkg, '.');
 		} catch (e) {}
 	}
 
@@ -66,60 +77,3 @@ export async function resolveModule(path, { readFile, hasFile, module, internal
 
 	return path;
 }
-
-/**
- * Get the best possible entry from a package.json that doesn't have an Export Map
- * @TODO this does not currently support {"browser":{"./foo.js":"./browser-foo.js"}}
- */
-function getLegacyEntry(pkg) {
-	const mainFields = [pkg.esmodules, pkg.modern, pkg.module, pkg['jsnext:main'], pkg.browser, pkg.main, 'index.js'];
-	const entry = mainFields.find(p => p && typeof p === 'string');
-	return '/' + entry.replace(/^\.?\//, '');
-}
-
-const ENV_KEYS = ['esmodules', 'import', 'module', 'require', 'browser', 'node', 'default'];
-
-/** Get the best resolution for an entry from an Export Map
- * @param {Object} exp `package.exports`
- * @param {string} entry `./foo` or `.`
- * @param {string[]} envKeys package environment keys
- * @returns {string | boolean} a resolved path, or a boolean indicating if the given entry is exposed
- */
-function resolveExportMap(exp, entry, envKeys) {
-	if (typeof exp === 'string') {
-		// {"exports":"./foo.js"}
-		// {"exports":{"./foo":"./foo.js"}}
-		return exp;
-	}
-	let isFileListing;
-	let isDirectoryExposed = false;
-
-	let fallbacks = [];
-	for (let i in exp) {
-		if (isFileListing === undefined) isFileListing = i[0] === '.';
-		if (isFileListing) {
-			// {"exports":{".":"./index.js"}}
-			if (i === entry) {
-				return resolveExportMap(exp[i], entry, envKeys);
-			}
-			if (!isDirectoryExposed && i.endsWith('/') && entry.startsWith(i)) {
-				isDirectoryExposed = true;
-			}
-		} else if (envKeys.includes(i)) {
-			// intentionally de-prioritize "require" and "default" keys
-			if (i === 'require' || i === 'default') {
-				fallbacks.push(i);
-			} else {
-				// {"exports":{"import":"./foo.js"}}
-				return resolveExportMap(exp[i], entry, envKeys);
-			}
-		}
-	}
-
-	// None of the in-order keys matched - fall back to require/default in the order specified
-	for (let i of fallbacks) {
-		return resolveExportMap(exp[i], entry, envKeys);
-	}
-
-	return isDirectoryExposed;
-}

packages/wmr/test/fixtures.test.js

@@ -482,6 +482,20 @@ describe('fixtures', () => {
 		});
 	});
 
+	describe('export-map', () => {
+		beforeEach(async () => {
+			await loadFixture('exports', env);
+			instance = await runWmrFast(env.tmp.path);
+			await env.page.goto(await instance.address);
+		});
+
+		it('should not pick node for a browser', async () => {
+			const test = await env.page.$('.test');
+			let text = test ? await test.evaluate(el => el.textContent) : null;
+			expect(text).toEqual('Browser implementation');
+		});
+	});
+
 	describe('package-exports', () => {
 		beforeEach(async () => {
 			await loadFixture('package-exports', env);
@@ -521,12 +535,12 @@ describe('fixtures', () => {
 				default: 'import'
 			});
 			expect(await env.page.evaluate(`import('/@npm/exports-fallbacks-defaultfirst')`)).toEqual({
-				default: 'import'
+				default: 'default'
 			});
 
 			// When import/module/browser isn't present (but a random other one is!), we fall back to require/default:
 			expect(await env.page.evaluate(`import('/@npm/exports-fallbacks-requirefallback')`)).toEqual({
-				default: 'require'
+				default: 'default'
 			});
 			expect(await env.page.evaluate(`import('/@npm/exports-fallbacks-defaultfallback')`)).toEqual({
 				default: 'default'

packages/wmr/test/fixtures/exports/node_modules/test/browser.mjs

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="utf8" />
+		<title>Exports</title>
+	</head>
+	<body>
+		<div id="root"></div>
+		<script src="./index.js" type="module"></script>
+	</body>
+</html>

packages/wmr/test/fixtures/exports/node_modules/test/node.mjs

@@ -0,0 +1,6 @@
+import value from 'test';
+
+const div = document.createElement('div');
+div.setAttribute('class', 'test');
+div.innerText = value;
+document.body.appendChild(div);