fix #2481: skip tsconfig paths inside node_modules

evanw · Jun 9, 2023 · b7426bd · b7426bd
1 parent 545964b
commit b7426bd
Show file tree

Hide file tree

Showing 5 changed files with 147 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -101,6 +101,10 @@
 
         This changes esbuild's behavior that applies `tsconfig.json` to all files in the subtree of the directory containing `tsconfig.json`. In version 0.12.7, esbuild started ignoring `tsconfig.json` files inside `node_modules` folders. The rationale is that people typically do this by mistake and that doing this intentionally is a rare use case that doesn't need to be supported. However, this change only applied to certain syntax-specific settings (e.g. `jsxFactory`) but did not apply to path resolution settings (e.g. `paths`). With this release, esbuild will now ignore all `tsconfig.json` files in `node_modules` instead of only ignoring certain settings.
 
+    * Ignore `tsconfig.json` when resolving paths within `node_modules` ([#2481](https://github.com/evanw/esbuild/issues/2481))
+
+        Previously fields in `tsconfig.json` related to path resolution (e.g. `paths`) were respected for all files in the subtree containing that `tsconfig.json` file, even within a nested `node_modules` subdirectory. This meant that a project's `paths` settings could potentially affect any bundled packages. With this release, esbuild will no longer use `tsconfig.json` settings during path resolution inside nested `node_modules` subdirectories.
+
     These changes are intended to improve esbuild's compatibility with `tsc` and reduce the number of unfortunate behaviors regarding `tsconfig.json` and esbuild.
 
 * Add a workaround for bugs in Safari 16.2 and earlier ([#3072](https://github.com/evanw/esbuild/issues/3072))

diff --git a/internal/bundler_tests/bundler_tsconfig_test.go b/internal/bundler_tests/bundler_tsconfig_test.go
@@ -2310,3 +2310,47 @@ func TestTsConfigExtendsArrayNested(t *testing.T) {
 		},
 	})
 }
+
+func TestTsConfigIgnoreInsideNodeModules(t *testing.T) {
+	tsconfig_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/src/main.ts": `
+				import { foo } from 'js-pkg'
+				import { bar } from 'ts-pkg'
+				import { foo as shimFoo, bar as shimBar } from 'pkg'
+				if (foo !== 'foo') throw 'fail: foo'
+				if (bar !== 'bar') throw 'fail: bar'
+				if (shimFoo !== 'shimFoo') throw 'fail: shimFoo'
+				if (shimBar !== 'shimBar') throw 'fail: shimBar'
+			`,
+			"/Users/user/project/shim.ts": `
+				export let foo = 'shimFoo'
+				export let bar = 'shimBar'
+			`,
+			"/Users/user/project/tsconfig.json": `{
+				"compilerOptions": {
+					"paths": {
+						"pkg": ["./shim"],
+					},
+				},
+			}`,
+			"/Users/user/project/node_modules/js-pkg/index.js": `
+				import { foo as pkgFoo } from 'pkg'
+				export let foo = pkgFoo
+			`,
+			"/Users/user/project/node_modules/ts-pkg/index.ts": `
+				import { bar as pkgBar } from 'pkg'
+				export let bar = pkgBar
+			`,
+			"/Users/user/project/node_modules/pkg/index.js": `
+				export let foo = 'foo'
+				export let bar = 'bar'
+			`,
+		},
+		entryPaths: []string{"/Users/user/project/src/main.ts"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/Users/user/project/out",
+		},
+	})
+}
diff --git a/internal/bundler_tests/snapshots/snapshots_tsconfig.txt b/internal/bundler_tests/snapshots/snapshots_tsconfig.txt
@@ -145,6 +145,33 @@ TestTsConfigExtendsWithExportsStarTrailing
 // Users/user/project/src/main.tsx
 console.log(/* @__PURE__ */ success("div", null));
 
+================================================================================
+TestTsConfigIgnoreInsideNodeModules
+---------- /Users/user/project/out/main.js ----------
+// Users/user/project/node_modules/pkg/index.js
+var foo = "foo";
+var bar = "bar";
+
+// Users/user/project/node_modules/js-pkg/index.js
+var foo2 = foo;
+
+// Users/user/project/node_modules/ts-pkg/index.ts
+var bar2 = bar;
+
+// Users/user/project/shim.ts
+var foo3 = "shimFoo";
+var bar3 = "shimBar";
+
+// Users/user/project/src/main.ts
+if (foo2 !== "foo")
+  throw "fail: foo";
+if (bar2 !== "bar")
+  throw "fail: bar";
+if (foo3 !== "shimFoo")
+  throw "fail: shimFoo";
+if (bar3 !== "shimBar")
+  throw "fail: shimBar";
+
 ================================================================================
 TestTsConfigJSX
 ---------- /Users/user/project/out.js ----------

diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
@@ -927,9 +927,13 @@ type dirInfo struct {
 	absRealPath           string        // If non-empty, this is the real absolute path resolving any symlinks
 	isNodeModules         bool          // Is the base name "node_modules"?
 	hasNodeModules        bool          // Is there a "node_modules" subdirectory?
+	isInsideNodeModules   bool          // Is this within a  "node_modules" subtree?
 }
 
 func (r resolverQuery) tsConfigForDir(dirInfo *dirInfo) *TSConfigJSON {
+	if dirInfo.isInsideNodeModules {
+		return nil
+	}
 	if r.tsConfigOverride != nil {
 		return r.tsConfigOverride
 	}
@@ -1329,6 +1333,7 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
 	base := r.fs.Base(path)
 	if base == "node_modules" {
 		info.isNodeModules = true
+		info.isInsideNodeModules = true
 	} else if entry, _ := entries.Get("node_modules"); entry != nil {
 		info.hasNodeModules = entry.Kind(r.fs) == fs.DirEntry
 	}
@@ -1338,6 +1343,9 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
 		info.enclosingPackageJSON = parentInfo.enclosingPackageJSON
 		info.enclosingBrowserScope = parentInfo.enclosingBrowserScope
 		info.enclosingTSConfigJSON = parentInfo.enclosingTSConfigJSON
+		if parentInfo.isInsideNodeModules {
+			info.isInsideNodeModules = true
+		}
 
 		// Make sure "absRealPath" is the real path of the directory (resolving any symlinks)
 		if !r.options.PreserveSymlinks {
@@ -1379,41 +1387,32 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
 		} else if entry, _ := entries.Get("jsconfig.json"); entry != nil && entry.Kind(r.fs) == fs.FileEntry {
 			tsConfigPath = r.fs.Join(path, "jsconfig.json")
 		}
-		if tsConfigPath != "" {
-			isInsideNodeModules := false
-			for dir := info; dir != nil; dir = dir.parent {
-				if dir.isNodeModules {
-					isInsideNodeModules = true
-					break
-				}
-			}
 
-			// Except don't do this if we're inside a "node_modules" directory. Package
-			// authors often publish their "tsconfig.json" files to npm because of
-			// npm's default-include publishing model and because these authors
-			// probably don't know about ".npmignore" files.
-			//
-			// People trying to use these packages with esbuild have historically
-			// complained that esbuild is respecting "tsconfig.json" in these cases.
-			// The assumption is that the package author published these files by
-			// accident.
-			//
-			// Ignoring "tsconfig.json" files inside "node_modules" directories breaks
-			// the use case of publishing TypeScript code and having it be transpiled
-			// for you, but that's the uncommon case and likely doesn't work with
-			// many other tools anyway. So now these files are ignored.
-			if !isInsideNodeModules {
-				var err error
-				info.enclosingTSConfigJSON, err = r.parseTSConfig(tsConfigPath, make(map[string]bool))
-				if err != nil {
-					if err == syscall.ENOENT {
-						r.log.AddError(nil, logger.Range{}, fmt.Sprintf("Cannot find tsconfig file %q",
-							PrettyPath(r.fs, logger.Path{Text: tsConfigPath, Namespace: "file"})))
-					} else if err != errParseErrorAlreadyLogged {
-						r.log.AddID(logger.MsgID_TSConfigJSON_Missing, logger.Debug, nil, logger.Range{},
-							fmt.Sprintf("Cannot read file %q: %s",
-								PrettyPath(r.fs, logger.Path{Text: tsConfigPath, Namespace: "file"}), err.Error()))
-					}
+		// Except don't do this if we're inside a "node_modules" directory. Package
+		// authors often publish their "tsconfig.json" files to npm because of
+		// npm's default-include publishing model and because these authors
+		// probably don't know about ".npmignore" files.
+		//
+		// People trying to use these packages with esbuild have historically
+		// complained that esbuild is respecting "tsconfig.json" in these cases.
+		// The assumption is that the package author published these files by
+		// accident.
+		//
+		// Ignoring "tsconfig.json" files inside "node_modules" directories breaks
+		// the use case of publishing TypeScript code and having it be transpiled
+		// for you, but that's the uncommon case and likely doesn't work with
+		// many other tools anyway. So now these files are ignored.
+		if tsConfigPath != "" && !info.isInsideNodeModules {
+			var err error
+			info.enclosingTSConfigJSON, err = r.parseTSConfig(tsConfigPath, make(map[string]bool))
+			if err != nil {
+				if err == syscall.ENOENT {
+					r.log.AddError(nil, logger.Range{}, fmt.Sprintf("Cannot find tsconfig file %q",
+						PrettyPath(r.fs, logger.Path{Text: tsConfigPath, Namespace: "file"})))
+				} else if err != errParseErrorAlreadyLogged {
+					r.log.AddID(logger.MsgID_TSConfigJSON_Missing, logger.Debug, nil, logger.Range{},
+						fmt.Sprintf("Cannot read file %q: %s",
+							PrettyPath(r.fs, logger.Path{Text: tsConfigPath, Namespace: "file"}), err.Error()))
 				}
 			}
 		}

diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js
@@ -146,6 +146,45 @@ tests.push(
   }),
 )
 
+// Check "tsconfig.json" behavior
+tests.push(
+  // See: https://github.com/evanw/esbuild/issues/2481
+  test(['main.ts', '--bundle', '--outfile=node.js'], {
+    'main.ts': `
+      import { foo } from 'js-pkg'
+      import { bar } from 'ts-pkg'
+      import { foo as shimFoo, bar as shimBar } from 'pkg'
+      if (foo !== 'foo') throw 'fail: foo'
+      if (bar !== 'bar') throw 'fail: bar'
+      if (shimFoo !== 'shimFoo') throw 'fail: shimFoo'
+      if (shimBar !== 'shimBar') throw 'fail: shimBar'
+    `,
+    'shim.ts': `
+      export let foo = 'shimFoo'
+      export let bar = 'shimBar'
+    `,
+    'tsconfig.json': `{
+      "compilerOptions": {
+        "paths": {
+          "pkg": ["./shim"],
+        },
+      },
+    }`,
+    'node_modules/js-pkg/index.js': `
+      import { foo as pkgFoo } from 'pkg'
+      export let foo = pkgFoo
+    `,
+    'node_modules/ts-pkg/index.ts': `
+      import { bar as pkgBar } from 'pkg'
+      export let bar = pkgBar
+    `,
+    'node_modules/pkg/index.js': `
+      export let foo = 'foo'
+      export let bar = 'bar'
+    `,
+  }),
+)
+
 // Test coverage for a special JSX error message
 tests.push(
   test(['example.jsx', '--outfile=node.js'], {