Skip to content

Commit

Permalink
fix #3341, fix #3608: sort .ts right after .js
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 26, 2024
1 parent f8ec300 commit 22a9cf5
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 8 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
46 changes: 46 additions & 0 deletions internal/bundler_tests/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
})
}
29 changes: 29 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_ts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 ----------
Expand Down
33 changes: 25 additions & 8 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down

0 comments on commit 22a9cf5

Please sign in to comment.