Skip to content

Commit

Permalink
fix #2473: yarn pnp exports in package.json
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 16, 2022
1 parent 5e085f5 commit 6a69a18
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 66 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
27 changes: 27 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
30 changes: 18 additions & 12 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 2 additions & 22 deletions internal/resolver/yarnpnp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, '/')

Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion internal/resolver/yarnpnp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
}
Expand Down
4 changes: 4 additions & 0 deletions require/yarnpnp/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/.pnp*
/.yarn*
/in.js
/out.js
11 changes: 11 additions & 0 deletions require/yarnpnp/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
3 changes: 3 additions & 0 deletions require/yarnpnp/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "@vue/tsconfig/tsconfig.json"
}
137 changes: 137 additions & 0 deletions require/yarnpnp/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 6
cacheKey: 8

"@tokenizer/token@npm:^0.3.0":
version: 0.3.0
resolution: "@tokenizer/token@npm:0.3.0"
checksum: 1d575d02d2a9f0c5a4ca5180635ebd2ad59e0f18b42a65f3d04844148b49b3db35cf00b6012a1af2d59c2ab3caca59451c5689f747ba8667ee586ad717ee58e1
languageName: node
linkType: hard

"@vue/tsconfig@npm:0.1.3":
version: 0.1.3
resolution: "@vue/tsconfig@npm:0.1.3"
peerDependencies:
"@types/node": "*"
peerDependenciesMeta:
"@types/node":
optional: true
checksum: 8150a24497a5348bc342c27afb38ad989de2ce8e94c349020628065d2a8df6837cb8bb3012f9161eea716487832612ac71b5f910d95bac41539ac6021d6bd88d
languageName: node
linkType: hard

"d3-array@npm:2 - 3":
version: 3.2.0
resolution: "d3-array@npm:3.2.0"
dependencies:
internmap: 1 - 2
checksum: e236f6670b60b64abb6c435da25b5cbbdc2c7c0decdbf9355bc4cf6803d6da4fa820b7b78b9cbd127edb493555934a9788d45084c2f39d7c2e1a2b7aa48264a4
languageName: node
linkType: hard

"d3-time@npm:3.0.0":
version: 3.0.0
resolution: "d3-time@npm:3.0.0"
dependencies:
d3-array: 2 - 3
checksum: 01646568ef01682550b7ee9f32394e4eb116a29515564861958871ed8de8fff02a25cd50dd8c4413921e6d9ecb8c8ce39be3266f655c8c18599fe58bcb253d60
languageName: node
linkType: hard

"internmap@npm:1 - 2":
version: 2.0.3
resolution: "internmap@npm:2.0.3"
checksum: 7ca41ec6aba8f0072fc32fa8a023450a9f44503e2d8e403583c55714b25efd6390c38a87161ec456bf42d7bc83aab62eb28f5aef34876b1ac4e60693d5e1d241
languageName: node
linkType: hard

"js-tokens@npm:^3.0.0 || ^4.0.0":
version: 4.0.0
resolution: "js-tokens@npm:4.0.0"
checksum: 8a95213a5a77deb6cbe94d86340e8d9ace2b93bc367790b260101d2f36a2eaf4e4e22d9fa9cf459b38af3a32fb4190e638024cf82ec95ef708680e405ea7cc78
languageName: node
linkType: hard

"loose-envify@npm:^1.1.0":
version: 1.4.0
resolution: "loose-envify@npm:1.4.0"
dependencies:
js-tokens: ^3.0.0 || ^4.0.0
bin:
loose-envify: cli.js
checksum: 6517e24e0cad87ec9888f500c5b5947032cdfe6ef65e1c1936a0c48a524b81e65542c9c3edc91c97d5bddc806ee2a985dbc79be89215d613b1de5db6d1cfe6f4
languageName: node
linkType: hard

"mime@npm:3.0.0":
version: 3.0.0
resolution: "mime@npm:3.0.0"
bin:
mime: cli.js
checksum: f43f9b7bfa64534e6b05bd6062961681aeb406a5b53673b53b683f27fcc4e739989941836a355eef831f4478923651ecc739f4a5f6e20a76487b432bfd4db928
languageName: node
linkType: hard

"peek-readable@npm:^5.0.0":
version: 5.0.0
resolution: "peek-readable@npm:5.0.0"
checksum: bef5ceb50586eb42e14efba274ac57ffe97f0ed272df9239ce029f688f495d9bf74b2886fa27847c706a9db33acda4b7d23bbd09a2d21eb4c2a54da915117414
languageName: node
linkType: hard

"react-dom@npm:18.2.0":
version: 18.2.0
resolution: "react-dom@npm:18.2.0"
dependencies:
loose-envify: ^1.1.0
scheduler: ^0.23.0
peerDependencies:
react: ^18.2.0
checksum: 7d323310bea3a91be2965f9468d552f201b1c27891e45ddc2d6b8f717680c95a75ae0bc1e3f5cf41472446a2589a75aed4483aee8169287909fcd59ad149e8cc
languageName: node
linkType: hard

"react@npm:18.2.0":
version: 18.2.0
resolution: "react@npm:18.2.0"
dependencies:
loose-envify: ^1.1.0
checksum: 88e38092da8839b830cda6feef2e8505dec8ace60579e46aa5490fc3dc9bba0bd50336507dc166f43e3afc1c42939c09fe33b25fae889d6f402721dcd78fca1b
languageName: node
linkType: hard

"root-workspace-0b6124@workspace:.":
version: 0.0.0-use.local
resolution: "root-workspace-0b6124@workspace:."
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
languageName: unknown
linkType: soft

"scheduler@npm:^0.23.0":
version: 0.23.0
resolution: "scheduler@npm:0.23.0"
dependencies:
loose-envify: ^1.1.0
checksum: d79192eeaa12abef860c195ea45d37cbf2bbf5f66e3c4dcd16f54a7da53b17788a70d109ee3d3dde1a0fd50e6a8fc171f4300356c5aee4fc0171de526bf35f8a
languageName: node
linkType: hard

"strtok3@npm:7.0.0":
version: 7.0.0
resolution: "strtok3@npm:7.0.0"
dependencies:
"@tokenizer/token": ^0.3.0
peek-readable: ^5.0.0
checksum: 2ebe7ad8f2aea611dec6742cf6a42e82764892a362907f7ce493faf334501bf981ce21c828dcc300457e6d460dc9c34d644ededb3b01dcb9e37559203cf1748c
languageName: node
linkType: hard

0 comments on commit 6a69a18

Please sign in to comment.