From 22a9cf5754d402aabfe75aeda0266c3a970b0ee1 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Fri, 26 Jan 2024 01:31:20 -0500 Subject: [PATCH] fix #3341, fix #3608: sort `.ts` right after `.js` --- CHANGELOG.md | 10 ++++ internal/bundler_tests/bundler_ts_test.go | 46 +++++++++++++++++++ .../bundler_tests/snapshots/snapshots_ts.txt | 29 ++++++++++++ internal/resolver/resolver.go | 33 +++++++++---- 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 926418dd088..35c48c6e37c 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/internal/bundler_tests/bundler_ts_test.go b/internal/bundler_tests/bundler_ts_test.go index a5b43e92b8e..1f9d2b0e67c 100644 --- a/internal/bundler_tests/bundler_ts_test.go +++ b/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", + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_ts.txt b/internal/bundler_tests/snapshots/snapshots_ts.txt index 7e3266ce144..69960ee039d 100644 --- a/internal/bundler_tests/snapshots/snapshots_ts.txt +++ b/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 ---------- diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index b7d8193e440..15c379ecaa7 100644 --- a/internal/resolver/resolver.go +++ b/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) + } } }