From 112b9aa32ea80eeef01cb91a8b415cfff08ef850 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 21 Sep 2024 17:16:09 -0400 Subject: [PATCH] fix #3915: stack overflow with yarn + tsconfig --- CHANGELOG.md | 4 ++ .../bundler_tests/bundler_tsconfig_test.go | 68 +++++++++++++++++++ .../snapshots/snapshots_tsconfig.txt | 6 ++ internal/resolver/resolver.go | 49 ++++++++++--- scripts/js-api-tests.js | 56 +++++++++++++++ 5 files changed, 175 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d86d5d48cc..36ee41b959b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/internal/bundler_tests/bundler_tsconfig_test.go b/internal/bundler_tests/bundler_tsconfig_test.go index 50ff57e6989..12d2a720771 100644 --- a/internal/bundler_tests/bundler_tsconfig_test.go +++ b/internal/bundler_tests/bundler_tsconfig_test.go @@ -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(
) + `, + "/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", + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_tsconfig.txt b/internal/bundler_tests/snapshots/snapshots_tsconfig.txt index 1e05b86c4f0..c96c6d86582 100644 --- a/internal/bundler_tests/snapshots/snapshots_tsconfig.txt +++ b/internal/bundler_tests/snapshots/snapshots_tsconfig.txt @@ -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 ---------- diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index ccc21309caf..3a4ae894db6 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -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 { @@ -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 @@ -2236,6 +2244,7 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON } absolute, ok, diffCase := r.finalizeImportsExportsResult( + finalizeImportsExportsNormal, dirInfoPackageJSON.absPath, conditions, *packageJSON.importsMap, packageJSON, resolvedPath, status, debug, "", "", "", @@ -2243,7 +2252,14 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON 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() @@ -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, @@ -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 } @@ -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 } @@ -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 } @@ -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, @@ -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 diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index d32d7c85ca7..534375a181a 100644 --- a/scripts/js-api-tests.js +++ b/scripts/js-api-tests.js @@ -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(
)`) + 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)); +})(); `) }, }