Skip to content

Commit

Permalink
fix #3915: stack overflow with yarn + tsconfig
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Sep 21, 2024
1 parent ed5a555 commit 112b9aa
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
./esbuild --version
```

* Handle Yarn Plug'n'Play stack overflow with `tsconfig.json` ([#3915](https://github.com/evanw/esbuild/issues/3915))

Previously a `tsconfig.json` file that `extends` another file in a package with an `exports` map could cause a stack overflow when Yarn's Plug'n'Play resolution was active. This edge case should work now starting with this release.

## 0.23.1

* Allow using the `node:` import prefix with `es*` targets ([#3821](https://github.com/evanw/esbuild/issues/3821))
Expand Down
68 changes: 68 additions & 0 deletions internal/bundler_tests/bundler_tsconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2749,3 +2749,71 @@ func TestTsconfigJsonConfigDirBaseURLInheritedPaths(t *testing.T) {
},
})
}

// https://github.com/evanw/esbuild/issues/3915
func TestTsconfigStackOverflowYarnPnP(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/entry.jsx": `
console.log(<div />)
`,
"/Users/user/project/tsconfig.json": `
{
"extends": "tsconfigs/config"
}
`,
"/Users/user/project/packages/tsconfigs/package.json": `
{
"exports": {
"./config": "./configs/tsconfig.json"
}
}
`,
"/Users/user/project/packages/tsconfigs/configs/tsconfig.json": `
{
"compilerOptions": {
"jsxFactory": "success"
}
}
`,
"/Users/user/project/.pnp.data.json": `
{
"packageRegistryData": [
[null, [
[null, {
"packageLocation": "./",
"packageDependencies": [
["tsconfigs", "virtual:some-path"]
],
"linkType": "SOFT"
}]
]],
["tsconfigs", [
["virtual:some-path", {
"packageLocation": "./packages/tsconfigs/",
"packageDependencies": [
["tsconfigs", "virtual:some-path"]
],
"packagePeers": [],
"linkType": "SOFT"
}],
["workspace:packages/tsconfigs", {
"packageLocation": "./packages/tsconfigs/",
"packageDependencies": [
["tsconfigs", "workspace:packages/tsconfigs"]
],
"linkType": "SOFT"
}]
]]
]
}
`,
},
entryPaths: []string{"/Users/user/project/entry.jsx"},
absWorkingDir: "/Users/user/project",
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}
6 changes: 6 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_tsconfig.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,12 @@ TestTsconfigRemoveUnusedImports
// Users/user/project/src/entry.ts
console.log(1);

================================================================================
TestTsconfigStackOverflowYarnPnP
---------- /Users/user/project/out.js ----------
// entry.jsx
console.log(/* @__PURE__ */ success("div", null));

================================================================================
TestTsconfigUnrecognizedTargetWarning
---------- /Users/user/project/out.js ----------
Expand Down
49 changes: 41 additions & 8 deletions internal/resolver/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1132,12 +1132,19 @@ func (r resolverQuery) dirInfoCached(path string) *dirInfo {

// Cache hit: stop now
if !ok {
// Update the cache to indicate failure. Even if the read failed, we don't
// want to retry again later. The directory is inaccessible so trying again
// is wasted. Doing this before calling "dirInfoUncached" prevents stack
// overflow in case this directory is recursively encountered again.
r.dirCache[path] = nil

// Cache miss: read the info
cached = r.dirInfoUncached(path)

// Update the cache unconditionally. Even if the read failed, we don't want to
// retry again later. The directory is inaccessible so trying again is wasted.
r.dirCache[path] = cached
// Only update the cache again on success
if cached != nil {
r.dirCache[path] = cached
}
}

if r.debugLogs != nil {
Expand Down Expand Up @@ -1294,7 +1301,8 @@ func (r resolverQuery) parseTSConfigFromSource(source logger.Source, visited map
if entry, _ := entries.Get("package.json"); entry != nil && entry.Kind(r.fs) == fs.FileEntry {
// Check the "exports" map
if packageJSON := r.parsePackageJSON(result.pkgDirPath); packageJSON != nil && packageJSON.exportsMap != nil {
if absolute, ok, _ := r.esmResolveAlgorithm(result.pkgIdent, "."+result.pkgSubpath, packageJSON, result.pkgDirPath, source.KeyPath.Text); ok {
if absolute, ok, _ := r.esmResolveAlgorithm(finalizeImportsExportsYarnPnPTSConfigExtends,
result.pkgIdent, "."+result.pkgSubpath, packageJSON, result.pkgDirPath, source.KeyPath.Text); ok {
base, err := r.parseTSConfig(absolute.Primary.Text, visited, configDir)
if result, shouldReturn := maybeFinishOurSearch(base, err, absolute.Primary.Text); shouldReturn {
return result
Expand Down Expand Up @@ -2236,14 +2244,22 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON
}

absolute, ok, diffCase := r.finalizeImportsExportsResult(
finalizeImportsExportsNormal,
dirInfoPackageJSON.absPath, conditions, *packageJSON.importsMap, packageJSON,
resolvedPath, status, debug,
"", "", "",
)
return absolute, ok, diffCase, nil
}

func (r resolverQuery) esmResolveAlgorithm(esmPackageName string, esmPackageSubpath string, packageJSON *packageJSON, absPkgPath string, absPath string) (PathPair, bool, *fs.DifferentCase) {
func (r resolverQuery) esmResolveAlgorithm(
kind finalizeImportsExportsKind,
esmPackageName string,
esmPackageSubpath string,
packageJSON *packageJSON,
absPkgPath string,
absPath string,
) (PathPair, bool, *fs.DifferentCase) {
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Looking for %q in \"exports\" map in %q", esmPackageSubpath, packageJSON.source.KeyPath.Text))
r.debugLogs.increaseIndent()
Expand Down Expand Up @@ -2278,6 +2294,7 @@ func (r resolverQuery) esmResolveAlgorithm(esmPackageName string, esmPackageSubp
resolvedPath, status, debug = r.esmHandlePostConditions(resolvedPath, status, debug)

return r.finalizeImportsExportsResult(
kind,
absPkgPath, conditions, *packageJSON.exportsMap, packageJSON,
resolvedPath, status, debug,
esmPackageName, esmPackageSubpath, absPath,
Expand Down Expand Up @@ -2358,7 +2375,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
if pkgDirInfo := r.dirInfoCached(result.pkgDirPath); pkgDirInfo != nil {
// Check the "exports" map
if packageJSON := pkgDirInfo.packageJSON; packageJSON != nil && packageJSON.exportsMap != nil {
absolute, ok, diffCase := r.esmResolveAlgorithm(result.pkgIdent, "."+result.pkgSubpath, packageJSON, pkgDirInfo.absPath, absPath)
absolute, ok, diffCase := r.esmResolveAlgorithm(finalizeImportsExportsNormal, result.pkgIdent, "."+result.pkgSubpath, packageJSON, pkgDirInfo.absPath, absPath)
return absolute, ok, diffCase, nil
}

Expand Down Expand Up @@ -2393,7 +2410,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
// Check for self-references
if dirInfoPackageJSON != nil {
if packageJSON := dirInfoPackageJSON.packageJSON; packageJSON.name == esmPackageName && packageJSON.exportsMap != nil {
absolute, ok, diffCase := r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON,
absolute, ok, diffCase := r.esmResolveAlgorithm(finalizeImportsExportsNormal, esmPackageName, esmPackageSubpath, packageJSON,
dirInfoPackageJSON.absPath, r.fs.Join(dirInfoPackageJSON.absPath, esmPackageSubpath))
return absolute, ok, diffCase, nil
}
Expand All @@ -2412,7 +2429,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
if pkgDirInfo := r.dirInfoCached(absPkgPath); pkgDirInfo != nil {
// Check the "exports" map
if packageJSON := pkgDirInfo.packageJSON; packageJSON != nil && packageJSON.exportsMap != nil {
absolute, ok, diffCase := r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON, absPkgPath, absPath)
absolute, ok, diffCase := r.esmResolveAlgorithm(finalizeImportsExportsNormal, esmPackageName, esmPackageSubpath, packageJSON, absPkgPath, absPath)
return absolute, ok, diffCase, nil, true
}

Expand Down Expand Up @@ -2524,7 +2541,15 @@ func (r resolverQuery) checkForBuiltInNodeModules(importPath string) (PathPair,
return PathPair{}, false, nil
}

type finalizeImportsExportsKind uint8

const (
finalizeImportsExportsNormal finalizeImportsExportsKind = iota
finalizeImportsExportsYarnPnPTSConfigExtends
)

func (r resolverQuery) finalizeImportsExportsResult(
kind finalizeImportsExportsKind,
absDirPath string,
conditions map[string]bool,
importExportMap pjMap,
Expand All @@ -2551,6 +2576,14 @@ func (r resolverQuery) finalizeImportsExportsResult(
r.debugLogs.addNote(fmt.Sprintf("The resolved path %q is exact", absResolvedPath))
}

// Avoid calling "dirInfoCached" recursively for "tsconfig.json" extends with Yarn PnP
if kind == finalizeImportsExportsYarnPnPTSConfigExtends {
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Resolved to %q", absResolvedPath))
}
return PathPair{Primary: logger.Path{Text: absResolvedPath, Namespace: "file"}}, true, nil
}

resolvedDirInfo := r.dirInfoCached(r.fs.Dir(absResolvedPath))
base := r.fs.Base(absResolvedPath)
extensionOrder := r.options.ExtensionOrder
Expand Down
56 changes: 56 additions & 0 deletions scripts/js-api-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3738,6 +3738,62 @@ import "after/alias";
// .yarn/cache/dep-zip/node_modules/dep/index.js
success();
})();
`)
},

// https://github.com/evanw/esbuild/issues/3915
async yarnPnP_stackOverflow({ esbuild, testDir }) {
const entry = path.join(testDir, 'entry.jsx')

await writeFileAsync(entry, `console.log(<div />)`)
await writeFileAsync(path.join(testDir, 'tsconfig.json'), `{ "extends": "tsconfigs/config" }`)
await mkdirAsync(path.join(testDir, 'packages/tsconfigs/configs'), { recursive: true })
await writeFileAsync(path.join(testDir, 'packages/tsconfigs/package.json'), `{ "exports": { "./config": "./configs/tsconfig.json" } }`)
await writeFileAsync(path.join(testDir, 'packages/tsconfigs/configs/tsconfig.json'), `{ "compilerOptions": { "jsxFactory": "success" } }`)

await writeFileAsync(path.join(testDir, '.pnp.data.json'), `{
"packageRegistryData": [
[null, [
[null, {
"packageLocation": "./",
"packageDependencies": [
["tsconfigs", "virtual:some-path"]
],
"linkType": "SOFT"
}]
]],
["tsconfigs", [
["virtual:some-path", {
"packageLocation": "./.yarn/__virtual__/tsconfigs-virtual-f56a53910e/1/packages/tsconfigs/",
"packageDependencies": [
["tsconfigs", "virtual:some-path"]
],
"packagePeers": [],
"linkType": "SOFT"
}],
["workspace:packages/tsconfigs", {
"packageLocation": "./packages/tsconfigs/",
"packageDependencies": [
["tsconfigs", "workspace:packages/tsconfigs"]
],
"linkType": "SOFT"
}]
]]
]
}`)

const value = await esbuild.build({
entryPoints: [entry],
bundle: true,
write: false,
absWorkingDir: testDir,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// entry.jsx
console.log(/* @__PURE__ */ success("div", null));
})();
`)
},
}
Expand Down

0 comments on commit 112b9aa

Please sign in to comment.