fix #3341, fix #3608: sort .ts right after .js

Author: evanw

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+* Reorder implicit file extensions within `node_modules` ([#3341](https://github.com/evanw/esbuild/issues/3341), [#3608](https://github.com/evanw/esbuild/issues/3608))
+
+    In [version 0.18.0](https://github.com/evanw/esbuild/releases/v0.18.0), esbuild changed the behavior of implicit file extensions within `node_modules` directories (i.e. in published packages) to prefer `.js` over `.ts` even when the `--resolve-extensions=` order prefers `.ts` over `.js` (which it does by default). However, doing that also accidentally made esbuild prefer `.css` over `.ts`, which caused problems for people that published packages containing both TypeScript and CSS in files with the same name.
+
+    With this release, esbuild will reorder TypeScript file extensions immediately after the last JavaScript file extensions in the implicit file extension order instead of putting them at the end of the order. Specifically the default implicit file extension order is `.tsx,.ts,.jsx,.js,.css,.json` which used to become `.jsx,.js,.css,.json,.tsx,.ts` in `node_modules` directories. With this release it will now become `.jsx,.js,.tsx,.ts,.css,.json` instead.
+
+    Why even rewrite the implicit file extension order at all? One reason is because the `.js` file is more likely to behave correctly than the `.ts` file. The behavior of the `.ts` file  may depend on `tsconfig.json` and the `tsconfig.json` file may not even be published, or may use `extends` to refer to a base `tsconfig.json` file that wasn't published. People can get into this situation when they forget to add all `.ts` files to their `.npmignore` file before publishing to npm. Picking `.js` over `.ts` helps make it more likely that resulting bundle will behave correctly.
+
 ## 0.19.12
 
 * The "preserve" JSX mode now preserves JSX text verbatim ([#3605](https://github.com/evanw/esbuild/issues/3605))

internal/bundler_tests/bundler_ts_test.go

@@ -2934,3 +2934,49 @@ func TestTSPrintNonFiniteNumberInsideWith(t *testing.T) {
 		},
 	})
 }
+
+func TestTSImportInNodeModulesNameCollisionWithCSS(t *testing.T) {
+	ts_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				import "pkg"
+			`,
+			"/node_modules/pkg/index.ts": `
+				import js_ts_css from "./js_ts_css"
+				import ts_css from "./ts_css"
+				import js_ts from "./js_ts"
+				js_ts_css()
+				ts_css()
+				js_ts()
+			`,
+			"/node_modules/pkg/js_ts_css.js": `
+				import './js_ts_css.css'
+				export default function() {}
+			`,
+			"/node_modules/pkg/js_ts_css.ts": `
+				TEST FAILED
+			`,
+			"/node_modules/pkg/js_ts_css.css": `
+				.js_ts_css {}
+			`,
+			"/node_modules/pkg/ts_css.ts": `
+				import './ts_css.css'
+				export default function() {}
+			`,
+			"/node_modules/pkg/ts_css.css": `
+				.ts_css {}
+			`,
+			"/node_modules/pkg/js_ts.js": `
+				export default function() {}
+			`,
+			"/node_modules/pkg/js_ts.ts": `
+				TEST FAILED
+			`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}

internal/bundler_tests/snapshots/snapshots_ts.txt

@@ -1464,6 +1464,35 @@ var value_copy = value;
 var foo = value_copy;
 console.log(foo);
 
+================================================================================
+TestTSImportInNodeModulesNameCollisionWithCSS
+---------- /out.js ----------
+// node_modules/pkg/js_ts_css.js
+function js_ts_css_default() {
+}
+
+// node_modules/pkg/ts_css.ts
+function ts_css_default() {
+}
+
+// node_modules/pkg/js_ts.js
+function js_ts_default() {
+}
+
+// node_modules/pkg/index.ts
+js_ts_css_default();
+ts_css_default();
+js_ts_default();
+
+---------- /out.css ----------
+/* node_modules/pkg/js_ts_css.css */
+.js_ts_css {
+}
+
+/* node_modules/pkg/ts_css.css */
+.ts_css {
+}
+
 ================================================================================
 TestTSImportMTS
 ---------- /out.js ----------

internal/resolver/resolver.go

@@ -248,17 +248,34 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
 		}
 	}
 
-	// Sort all JavaScript file extensions after TypeScript file extensions
-	// for imports of files inside of "node_modules" directories
+	// Sort all TypeScript file extensions after all JavaScript file extensions
+	// for imports of files inside of "node_modules" directories. But insert
+	// the TypeScript file extensions right after the last JavaScript file
+	// extension instead of at the end so that they might come before the
+	// first CSS file extension, which is important to people that publish
+	// TypeScript and CSS code to npm with the same file names for both.
 	nodeModulesExtensionOrder := make([]string, 0, len(options.ExtensionOrder))
-	for _, ext := range options.ExtensionOrder {
-		if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
-			nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+	split := 0
+	for i, ext := range options.ExtensionOrder {
+		if loader, ok := options.ExtensionToLoader[ext]; ok && loader == config.LoaderJS || loader == config.LoaderJSX {
+			split = i + 1 // Split after the last JavaScript extension
 		}
 	}
-	for _, ext := range options.ExtensionOrder {
-		if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsTypeScript() {
-			nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+	if split != 0 { // Only do this if there are any JavaScript extensions
+		for _, ext := range options.ExtensionOrder[:split] { // Non-TypeScript extensions before the split
+			if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
+				nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+			}
+		}
+		for _, ext := range options.ExtensionOrder { // All TypeScript extensions
+			if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsTypeScript() {
+				nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+			}
+		}
+		for _, ext := range options.ExtensionOrder[split:] { // Non-TypeScript extensions after the split
+			if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
+				nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+			}
 		}
 	}