fix #2473: yarn pnp exports in package.json

Author: evanw

.github/workflows/ci.yml

@@ -133,6 +133,10 @@ jobs:
         if: matrix.os != 'ubuntu-latest'
         run: node scripts/wasm-tests.js
 
+      - name: Yarn PnP tests (non-Windows)
+        if: matrix.os != 'windows-latest'
+        run: make test-yarnpnp
+
       - name: Sucrase Tests
         if: matrix.os == 'ubuntu-latest'
         run: make test-sucrase

CHANGELOG.md

@@ -38,6 +38,10 @@
 
     For context: JavaScript files recently allowed using a [hashbang comment](https://github.com/tc39/proposal-hashbang), which starts with `#!` and which must start at the very first character of the file. It allows Unix systems to execute the file directly as a script without needing to prefix it by the `node` command. This comment typically has the value `#!/usr/bin/env node`. Hashbang comments will be a part of ES2023 when it's released next year.
 
+* Fix `exports` maps with Yarn PnP path resolution ([#2473](https://github.com/evanw/esbuild/issues/2473))
+
+    The Yarn PnP specification says that to resolve a package path, you first resolve it to the absolute path of a directory, and then you run node's module resolution algorithm on it. Previously esbuild followed this part of the specification. However, doing this means that `exports` in `package.json` is not respected because node's module resolution algorithm doesn't interpret `exports` for absolute paths. So with this release, esbuild will now use a modified algorithm that deviates from both specifications but that should hopefully behave more similar to what Yarn actually does: node's module resolution algorithm is run with the original import path but starting from the directory returned by Yarn PnP.
+
 ## 0.15.3
 
 * Change the Yarn PnP manifest to a singleton ([#2463](https://github.com/evanw/esbuild/issues/2463))

Makefile

@@ -201,6 +201,32 @@ test-e2e-yarn-berry:
 	# Clean up
 	rm -fr e2e-yb
 
+require/yarnpnp/.yarn/releases/yarn-3.2.2.cjs: require/yarnpnp/package.json require/yarnpnp/yarn.lock
+	rm -fr require/yarnpnp/.pnp* require/yarnpnp/.yarn*
+	which yarn || npm i -g yarn
+	cd require/yarnpnp && yarn set version 3.2.2
+
+require/yarnpnp/.yarn/cache: require/yarnpnp/.yarn/releases/yarn-3.2.2.cjs
+	cd require/yarnpnp && yarn install
+
+require/yarnpnp/in.js: Makefile
+	@echo 'console.log("Running Yarn PnP tests...")' > require/yarnpnp/in.js
+	@echo 'import * as rd from "react-dom"; if (rd.version !== "18.2.0") throw "❌ react-dom"' >> require/yarnpnp/in.js
+	@echo 'import * as s3 from "strtok3"; if (!s3.fromFile) throw "❌ strtok3"' >> require/yarnpnp/in.js
+	@echo 'import * as d3 from "d3-time"; if (!d3.utcDay) throw "❌ d3-time"' >> require/yarnpnp/in.js
+	@echo 'import * as mm from "mime"; if (mm.getType("txt") !== "text/plain") throw "❌ mime"' >> require/yarnpnp/in.js
+	@echo 'console.log("✅ Yarn PnP tests passed")' >> require/yarnpnp/in.js
+
+test-yarnpnp: esbuild require/yarnpnp/in.js | require/yarnpnp/.yarn/cache
+	cd require/yarnpnp && ../../esbuild --bundle in.js --log-level=debug --platform=node --outfile=out.js && node out.js
+
+# Note: This is currently failing due to a bug in Yarn that generates invalid
+# file handles. I should update the Yarn version and then enable this test
+# when a new version of Yarn is released that fixes this bug. This test is
+# disabled for now.
+test-yarnpnp-wasm: platform-wasm require/yarnpnp/in.js | require/yarnpnp/.yarn/cache
+	cd require/yarnpnp && yarn node ../../npm/esbuild-wasm/bin/esbuild --bundle in.js --log-level=debug --platform=node --outfile=out.js && node out.js
+
 # Note: This used to only be rebuilt when "version.txt" was newer than
 # "cmd/esbuild/version.go", but that caused the publishing script to publish
 # invalid builds in the case when the publishing script failed once, the change
@@ -575,6 +601,7 @@ clean:
 	rm -rf require/*/bench/
 	rm -rf require/*/demo/
 	rm -rf require/*/node_modules/
+	rm -rf require/yarnpnp/.pnp* require/yarnpnp/.yarn*
 	go clean -testcache ./internal/...
 
 # This also cleans directories containing cached code from other projects

internal/bundler/bundler.go

@@ -510,7 +510,11 @@ func ResolveFailureErrorTextSuggestionNotes(
 	}
 
 	if platform != config.PlatformNode {
-		if _, ok := resolver.BuiltInNodeModules[path]; ok {
+		pkg := path
+		if strings.HasPrefix(pkg, "node:") {
+			pkg = pkg[5:]
+		}
+		if resolver.BuiltInNodeModules[pkg] {
 			var how string
 			switch logger.API {
 			case logger.CLIAPI:

internal/resolver/resolver.go

@@ -721,15 +721,6 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) {
 }
 
 func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *dirInfo, importPath string) *ResolveResult {
-	// If Yarn PnP is active, use it to rewrite the path
-	if r.pnpManifest != nil {
-		if result, ok := r.pnpResolve(importPath, sourceDirInfo.absPath, r.pnpManifest); ok {
-			importPath = result // Continue with the module resolution algorithm from node.js
-		} else {
-			return nil // This is a module resolution error
-		}
-	}
-
 	// This implements the module resolution algorithm from node.js, which is
 	// described here: https://nodejs.org/api/modules.html#modules_all_together
 	var result ResolveResult
@@ -984,13 +975,13 @@ func (r resolverQuery) parseTSConfig(file string, visited map[string]bool) (*TSC
 						}
 
 						absPath = r.fs.Join(current, ".pnp.cjs")
-						if json := r.extractYarnPnPDataFromJSON(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
+						if json := r.tryToExtractYarnPnPDataFromJS(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
 							pnpData = compileYarnPnPData(absPath, current, json)
 							break
 						}
 
 						absPath = r.fs.Join(current, ".pnp.js")
-						if json := r.extractYarnPnPDataFromJSON(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
+						if json := r.tryToExtractYarnPnPDataFromJS(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
 							pnpData = compileYarnPnPData(absPath, current, json)
 							break
 						}
@@ -1006,7 +997,7 @@ func (r resolverQuery) parseTSConfig(file string, visited map[string]bool) (*TSC
 			}
 
 			if pnpData != nil {
-				if result, ok := r.pnpResolve(extends, fileDir, pnpData); ok {
+				if result, ok := r.resolveToUnqualified(extends, fileDir, pnpData); ok {
 					extends = result // Continue with the module resolution algorithm from node.js
 				}
 			}
@@ -1991,6 +1982,21 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
 		}
 	}
 
+	// If Yarn PnP is active, use it to rewrite the path
+	if r.pnpManifest != nil {
+		if result, ok := r.resolveToUnqualified(importPath, dirInfo.absPath, r.pnpManifest); ok {
+			if resultDirInfo := r.dirInfoCached(result); resultDirInfo != nil {
+				// Continue with the module resolution algorithm from node.js but
+				// pretend that the request started from wherever Yarn resolved us to.
+				// This isn't in the Yarn PnP specification but it's what Yarn does:
+				// https://github.com/evanw/esbuild/issues/2473#issuecomment-1216774461
+				dirInfo = resultDirInfo
+			}
+		} else {
+			return PathPair{}, false, nil // This is a module resolution error
+		}
+	}
+
 	// Find the parent directory with the "package.json" file
 	dirInfoPackageJSON := dirInfo
 	for dirInfoPackageJSON != nil && dirInfoPackageJSON.packageJSON == nil {

internal/resolver/yarnpnp.go

@@ -77,28 +77,6 @@ type pnpPackageLocatorByLocation struct {
 	discardFromLookup bool
 }
 
-// Note: If this returns successfully then the node module resolution algorithm
-// (i.e. NM_RESOLVE in the Yarn PnP specification) is always run afterward
-func (r resolverQuery) pnpResolve(specifier string, parentURL string, parentManifest *pnpData) (string, bool) {
-	// If specifier is a Node.js builtin, then
-	if BuiltInNodeModules[specifier] {
-		// Set resolved to specifier itself and return it
-		return specifier, true
-	}
-
-	// Otherwise, if `specifier` is either an absolute path or a path prefixed with "./" or "../", then
-	if r.fs.IsAbs(specifier) || strings.HasPrefix(specifier, "./") || strings.HasPrefix(specifier, "../") {
-		// Set resolved to NM_RESOLVE(specifier, parentURL) and return it
-		return specifier, true
-	}
-
-	// Otherwise,
-	// Note: specifier is now a bare identifier
-	// Let unqualified be RESOLVE_TO_UNQUALIFIED(specifier, parentURL)
-	// Set resolved to NM_RESOLVE(unqualified, parentURL)
-	return r.resolveToUnqualified(specifier, parentURL, parentManifest)
-}
-
 func parseBareIdentifier(specifier string) (ident string, modulePath string, ok bool) {
 	slash := strings.IndexByte(specifier, '/')
 
@@ -135,6 +113,8 @@ func parseBareIdentifier(specifier string) (ident string, modulePath string, ok
 	return
 }
 
+// Note: If this returns successfully then the node module resolution algorithm
+// (i.e. NM_RESOLVE in the Yarn PnP specification) is always run afterward
 func (r resolverQuery) resolveToUnqualified(specifier string, parentURL string, manifest *pnpData) (string, bool) {
 	// Let resolved be undefined
 

internal/resolver/yarnpnp_test.go

@@ -69,7 +69,7 @@ func TestYarnPnP(t *testing.T) {
 				t.Run(current.It, func(t *testing.T) {
 					rr := NewResolver(fs.MockFS(nil, fs.MockUnix), logger.NewDeferLog(logger.DeferLogNoVerboseOrDebug, nil), nil, config.Options{})
 					r := resolverQuery{resolver: rr.(*resolver)}
-					result, ok := r.pnpResolve(current.Imported, current.Importer, manifest)
+					result, ok := r.resolveToUnqualified(current.Imported, current.Importer, manifest)
 					if !ok {
 						result = "error!"
 					}

require/yarnpnp/.gitignore

@@ -0,0 +1,4 @@
+/.pnp*
+/.yarn*
+/in.js
+/out.js

require/yarnpnp/package.json

@@ -0,0 +1,11 @@
+{
+  "packageManager": "yarn@3.2.2",
+  "dependencies": {
+    "@vue/tsconfig": "0.1.3",
+    "d3-time": "3.0.0",
+    "mime": "3.0.0",
+    "react": "18.2.0",
+    "react-dom": "18.2.0",
+    "strtok3": "7.0.0"
+  }
+}

require/yarnpnp/tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "extends": "@vue/tsconfig/tsconfig.json"
+}