Skip to content

Commit

Permalink
fix #2463: change yarn pnp manifest to a singleton
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Aug 14, 2022
1 parent 6fd8736 commit 44a7a61
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 63 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

* Change the Yarn PnP manifest to a singleton ([#2463](https://github.com/evanw/esbuild/issues/2463))

Previously esbuild searched for the Yarn PnP manifest in the parent directories of each file. But with Yarn's `enableGlobalCache` setting it's possible to configure Yarn PnP's implementation to reach outside of the directory subtree containing the Yarn PnP manifest. This was causing esbuild to fail to bundle projects with the `enableGlobalCache` setting enabled.

To handle this case, *esbuild will now only search for the Yarn PnP manifest in the current working directory of the esbuild process*. If you're using esbuild's CLI, this means you will now have to `cd` into the appropriate directory first. If you're using esbuild's API, you can override esbuild's value for the current working directory with the `absWorkingDir` API option.

* Fix Yarn PnP resolution failures due to backslashes in paths on Windows ([#2462](https://github.com/evanw/esbuild/issues/2462))

Previously dependencies of a Yarn PnP virtual dependency failed to resolve on Windows. This was because Windows uses `\` instead of `/` as a path separator, and the path manipulation algorithms used for Yarn PnP expected `/`. This release converts `\` into `/` in Windows paths, which fixes this issue.
Expand Down
122 changes: 74 additions & 48 deletions internal/resolver/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ type resolver struct {
// all parent directories
dirCache map[string]*dirInfo

pnpManifestWasChecked bool
pnpManifest *pnpData

options config.Options

// This mutex serves two purposes. First of all, it guards access to "dirCache"
Expand Down Expand Up @@ -254,6 +257,8 @@ func NewResolver(fs fs.FS, log logger.Log, caches *cache.CacheSet, options confi
esmConditionsRequire[key] = true
}

fs.Cwd()

return &resolver{
fs: fs,
log: log,
Expand Down Expand Up @@ -409,6 +414,30 @@ func (rr *resolver) Resolve(sourceDir string, importPath string, kind ast.Import
defer r.mutex.Unlock()
sourceDirInfo := r.loadModuleSuffixesForSourceDir(sourceDir)

// Check for the Yarn PnP manifest if it hasn't already been checked for
if !r.pnpManifestWasChecked {
r.pnpManifestWasChecked = true

// Use the current working directory to find the Yarn PnP manifest. We
// can't necessarily use the entry point locations because the entry
// point locations aren't necessarily file paths. For example, they could
// be HTTP URLs that will be handled by a plugin.
for dirInfo := r.dirInfoCached(r.fs.Cwd()); dirInfo != nil; dirInfo = dirInfo.parent {
if absPath := dirInfo.pnpManifestAbsPath; absPath != "" {
if strings.HasSuffix(absPath, ".json") {
if json := r.extractYarnPnPDataFromJSON(absPath, pnpReportErrorsAboutMissingFiles); json.Data != nil {
r.pnpManifest = compileYarnPnPData(absPath, r.fs.Dir(absPath), json)
}
} else {
if json := r.tryToExtractYarnPnPDataFromJS(absPath, pnpReportErrorsAboutMissingFiles); json.Data != nil {
r.pnpManifest = compileYarnPnPData(absPath, r.fs.Dir(absPath), json)
}
}
break
}
}
}

result := r.resolveWithoutSymlinks(sourceDir, sourceDirInfo, importPath)
if result == nil {
// If resolution failed, try again with the URL query and/or hash removed
Expand Down Expand Up @@ -692,15 +721,12 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) {
}

func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *dirInfo, importPath string) *ResolveResult {
// Find the parent directory with the Yarn PnP data
for info := sourceDirInfo; info != nil; info = info.parent {
if info.pnpData != nil {
if result, ok := r.pnpResolve(importPath, sourceDirInfo.absPath, info.pnpData); ok {
importPath = result // Continue with the module resolution algorithm from node.js
} else {
return nil // This is a module resolution error
}
break
// 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
}
}

Expand Down Expand Up @@ -860,8 +886,8 @@ type dirInfo struct {

// All relevant information about this directory
absPath string
pnpManifestAbsPath string
entries fs.DirEntries
pnpData *pnpData
packageJSON *packageJSON // Is there a "package.json" file in this directory?
enclosingPackageJSON *packageJSON // Is there a "package.json" file in this directory or a parent directory?
enclosingTSConfigJSON *TSConfigJSON // Is there a "tsconfig.json" file in this directory or a parent directory?
Expand Down Expand Up @@ -944,38 +970,45 @@ func (r resolverQuery) parseTSConfig(file string, visited map[string]bool) (*TSC

// Check for a Yarn PnP manifest and use that to rewrite the path
if IsPackagePath(extends) {
current := fileDir
for {
if _, _, ok := fs.ParseYarnPnPVirtualPath(current); !ok {
var pnpData *pnpData
absPath := r.fs.Join(current, ".pnp.data.json")
if json := r.extractYarnPnPDataFromJSON(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
pnpData = compileYarnPnPData(absPath, current, json)
} else {
absPath := r.fs.Join(current, ".pnp.cjs")
pnpData := r.pnpManifest

// If we haven't loaded the Yarn PnP manifest yet, try to find one
if pnpData == nil {
current := fileDir
for {
if _, _, ok := fs.ParseYarnPnPVirtualPath(current); !ok {
absPath := r.fs.Join(current, ".pnp.data.json")
if json := r.extractYarnPnPDataFromJSON(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
pnpData = compileYarnPnPData(absPath, current, json)
} else {
absPath := r.fs.Join(current, ".pnp.js")
if json := r.extractYarnPnPDataFromJSON(absPath, pnpIgnoreErrorsAboutMissingFiles); json.Data != nil {
pnpData = compileYarnPnPData(absPath, current, json)
}
break
}
}
if pnpData != nil {
if result, ok := r.pnpResolve(extends, current, pnpData); ok {
extends = result // Continue with the module resolution algorithm from node.js

absPath = r.fs.Join(current, ".pnp.cjs")
if json := r.extractYarnPnPDataFromJSON(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 {
pnpData = compileYarnPnPData(absPath, current, json)
break
}
}

// Go to the parent directory, stopping at the file system root
next := r.fs.Dir(current)
if current == next {
break
}
current = next
}
}

// Go to the parent directory, stopping at the file system root
next := r.fs.Dir(current)
if current == next {
break
if pnpData != nil {
if result, ok := r.pnpResolve(extends, fileDir, pnpData); ok {
extends = result // Continue with the module resolution algorithm from node.js
}
current = next
}
}

Expand Down Expand Up @@ -1251,21 +1284,14 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
//
// /project/.yarn/__virtual__/pkg/1/.yarn/__virtual__/pkg/1/bar
//
if _, _, ok := fs.ParseYarnPnPVirtualPath(path); !ok {
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.data.json")
if json := r.extractYarnPnPDataFromJSON(absPath, pnpReportErrorsAboutMissingFiles); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.cjs")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, pnpReportErrorsAboutMissingFiles); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.js")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, pnpReportErrorsAboutMissingFiles); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
if r.pnpManifest == nil {
if _, _, ok := fs.ParseYarnPnPVirtualPath(path); !ok {
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
info.pnpManifestAbsPath = r.fs.Join(path, ".pnp.data.json")
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
info.pnpManifestAbsPath = r.fs.Join(path, ".pnp.cjs")
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
info.pnpManifestAbsPath = r.fs.Join(path, ".pnp.js")
}
}
}
Expand Down
38 changes: 23 additions & 15 deletions scripts/js-api-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2774,15 +2774,16 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_pnp_data_json/.yarn/cache/left-pad/index.js
// .yarn/cache/left-pad/index.js
function left_pad_default() {
}
// scripts/.js-api-tests/yarnPnP_pnp_data_json/entry.js
// entry.js
console.log(left_pad_default());
})();
`)
Expand Down Expand Up @@ -2840,15 +2841,16 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_pnp_js_object_literal/.yarn/cache/left-pad/index.js
// .yarn/cache/left-pad/index.js
function left_pad_default() {
}
// scripts/.js-api-tests/yarnPnP_pnp_js_object_literal/entry.js
// entry.js
console.log(left_pad_default());
})();
`)
Expand Down Expand Up @@ -2906,15 +2908,16 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_string_literal/.yarn/cache/left-pad/index.js
// .yarn/cache/left-pad/index.js
function left_pad_default() {
}
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_string_literal/entry.js
// entry.js
console.log(left_pad_default());
})();
`)
Expand Down Expand Up @@ -2974,15 +2977,16 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_identifier/.yarn/cache/left-pad/index.js
// .yarn/cache/left-pad/index.js
function left_pad_default() {
}
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_identifier/entry.js
// entry.js
console.log(left_pad_default());
})();
`)
Expand Down Expand Up @@ -3045,17 +3049,18 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/bar/index.js
// __virtual__/whatever/0/bar/index.js
var bar_default = "bar";
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/foo/index.js
// __virtual__/whatever/0/foo/index.js
var foo_default = "foo" + bar_default;
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/entry.js
// entry.js
console.log(foo_default);
})();
`)
Expand Down Expand Up @@ -3107,13 +3112,14 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
var __pow = Math.pow;
// scripts/.js-api-tests/yarnPnP_tsconfig/entry.js
// entry.js
x = __pow(x, 2);
})();
`)
Expand Down Expand Up @@ -3161,14 +3167,15 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_indexJs/foo/index.js
// foo/index.js
var foo_default = success;
// scripts/.js-api-tests/yarnPnP_indexJs/entry.js
// entry.js
foo_default();
})();
`)
Expand Down Expand Up @@ -3242,11 +3249,12 @@ require("/assets/file.png");
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_depOfVirtual/.yarn/cache/dep/index.js
// .yarn/cache/dep/index.js
success();
})();
`)
Expand Down

0 comments on commit 44a7a61

Please sign in to comment.